From 60f4ea42153c8975ac568209e9e684ed74275f71 Mon Sep 17 00:00:00 2001 From: hengsin Date: Tue, 11 Aug 2020 20:55:03 +0800 Subject: [PATCH] =?UTF-8?q?IDEMPIERE-4406=20Performance:=20PO=20Cache=20sh?= =?UTF-8?q?ould=20not=20always=20reset=20all=20entr=E2=80=A6=20(#212)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record PO update - reset cache by record id * IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record refine unit test * IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record add cache reset fix for delete * IDEMPIERE-4406 Performance: PO Cache should not always reset all entries after update of one record Fix exception when cache is empty Expose hidden cache reset exception --- .../src/org/compiere/model/PO.java | 4 +- .../src/org/compiere/util/CCache.java | 17 +++- .../src/org/compiere/util/CacheInterface.java | 3 +- .../src/org/compiere/util/CacheMgt.java | 14 ++- .../idempiere/test/performance/CacheTest.java | 91 +++++++++++++++++++ 5 files changed, 117 insertions(+), 12 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/model/PO.java b/org.adempiere.base/src/org/compiere/model/PO.java index 51c62db597..8fd55a8b57 100644 --- a/org.adempiere.base/src/org/compiere/model/PO.java +++ b/org.adempiere.base/src/org/compiere/model/PO.java @@ -2384,7 +2384,7 @@ public abstract class PO m_createNew = false; } if (!newRecord) { - CacheMgt.get().reset(p_info.getTableName()); + CacheMgt.get().reset(p_info.getTableName(), get_ID()); MRecentItem.clearLabel(p_info.getAD_Table_ID(), get_ID()); } else if (get_ID() > 0 && success) CacheMgt.get().newRecord(p_info.getTableName(), get_ID()); @@ -3542,7 +3542,7 @@ public abstract class PO int size = p_info.getColumnCount(); m_oldValues = new Object[size]; m_newValues = new Object[size]; - CacheMgt.get().reset(p_info.getTableName()); + CacheMgt.get().reset(p_info.getTableName(), Record_ID); } } finally diff --git a/org.adempiere.base/src/org/compiere/util/CCache.java b/org.adempiere.base/src/org/compiere/util/CCache.java index 73bb480ee0..877ba2960f 100644 --- a/org.adempiere.base/src/org/compiere/util/CCache.java +++ b/org.adempiere.base/src/org/compiere/util/CCache.java @@ -22,6 +22,7 @@ import java.io.Serializable; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; @@ -427,12 +428,18 @@ public class CCache implements CacheInterface, Map, Serializable public int reset(int recordId) { if (recordId <= 0) return reset(); - - if (!nullList.isEmpty()) { - if (nullList.remove(recordId)) return 1; + + Iterator iterator = cache.keySet().iterator(); + K firstKey = iterator.hasNext() ? iterator.next() : null; + if (firstKey != null && firstKey instanceof Integer) { + if (!nullList.isEmpty()) { + if (nullList.remove(recordId)) return 1; + } + V removed = cache.remove(recordId); + return removed != null ? 1 : 0; + } else { + return reset(); } - V removed = cache.remove(recordId); - return removed != null ? 1 : 0; } @Override diff --git a/org.adempiere.base/src/org/compiere/util/CacheInterface.java b/org.adempiere.base/src/org/compiere/util/CacheInterface.java index 5fb59afb9e..badb324342 100644 --- a/org.adempiere.base/src/org/compiere/util/CacheInterface.java +++ b/org.adempiere.base/src/org/compiere/util/CacheInterface.java @@ -31,7 +31,8 @@ public interface CacheInterface public int reset(); /** - * Reset Cache + * Reset Cache by record id + * @param recordId * @return number of items reset */ public int reset(int recordId); diff --git a/org.adempiere.base/src/org/compiere/util/CacheMgt.java b/org.adempiere.base/src/org/compiere/util/CacheMgt.java index c2671ce25b..1c13de55c2 100644 --- a/org.adempiere.base/src/org/compiere/util/CacheMgt.java +++ b/org.adempiere.base/src/org/compiere/util/CacheMgt.java @@ -176,9 +176,15 @@ public class CacheMgt total += i.get(); } } catch (InterruptedException e) { - e.printStackTrace(); + throw new RuntimeException(e); } catch (ExecutionException e) { - e.printStackTrace(); + if (e.getCause() != null) + if (e.getCause() instanceof RuntimeException) + throw (RuntimeException)e.getCause(); + else + throw new RuntimeException(e.getCause()); + else + throw new RuntimeException(e); } return total; } else { @@ -263,9 +269,9 @@ public class CacheMgt } /** - * @return + * @return cache instances */ - protected synchronized CacheInterface[] getInstancesAsArray() { + public synchronized CacheInterface[] getInstancesAsArray() { return m_instances.toArray(new CacheInterface[0]); } diff --git a/org.idempiere.test/src/org/idempiere/test/performance/CacheTest.java b/org.idempiere.test/src/org/idempiere/test/performance/CacheTest.java index 7e534f2282..5e1d340ccc 100644 --- a/org.idempiere.test/src/org/idempiere/test/performance/CacheTest.java +++ b/org.idempiere.test/src/org/idempiere/test/performance/CacheTest.java @@ -24,15 +24,20 @@ **********************************************************************/ package org.idempiere.test.performance; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import org.compiere.model.I_AD_Table; import org.compiere.model.MColumn; import org.compiere.model.MOrder; +import org.compiere.model.MProduct; import org.compiere.model.MRefTable; import org.compiere.model.MTable; import org.compiere.model.MWarehouse; import org.compiere.model.MZoomCondition; +import org.compiere.util.CCache; +import org.compiere.util.CacheInterface; import org.compiere.util.CacheMgt; import org.compiere.util.Env; import org.idempiere.test.AbstractTestCase; @@ -80,4 +85,90 @@ public class CacheTest extends AbstractTestCase { table2 = refTable.getAD_Table(); assertTrue(table == table2); } + + @SuppressWarnings({"unchecked", "rawtypes"}) + @Test + public void testPOCacheAfterUpdate() { + int mulch = 137; + int oak = 123; + //init cache + MProduct p1 = MProduct.get(Env.getCtx(), mulch); + CCache pc = null; + CacheInterface[] cis = CacheMgt.get().getInstancesAsArray(); + //find product cache instance + for(CacheInterface ci : cis) { + if (ci instanceof CCache) { + CCache ccache = (CCache) ci; + if (ccache.getName().equals(ccache.getTableName()) && ccache.getTableName().equals(MProduct.Table_Name)) { + if (ccache.containsKey(mulch)) { + pc = ccache; + break; + } + } + } + } + + if (pc == null) + fail("Product cache instance missing"); + + //second get, hit should increase + long hit = pc.getHit(); + p1 = MProduct.get(Env.getCtx(), mulch); + assertEquals(mulch, p1.getM_Product_ID()); + assertTrue(pc.getHit() > hit, "Second get of product Mulch, cache hit should increase"); + + //first get for p2, miss should increase + long miss = pc.getMiss(); + MProduct p2 = MProduct.get(Env.getCtx(), oak); + assertEquals(oak, p2.getM_Product_ID()); + assertTrue(pc.getMiss() > miss, "First get of product Oak, cache miss should increase"); + + //second get for p2, hit should increase + hit = pc.getHit(); + p2 = MProduct.get(Env.getCtx(), oak); + assertEquals(oak, p2.getM_Product_ID()); + assertTrue(pc.getHit() > hit, "Second get of product Oak, cache hit should increase"); + + p2.set_TrxName(getTrxName()); + p2.setDescription("Test Update @ " + System.currentTimeMillis()); + p2.saveEx(); + + //get after p2 update, miss should increase + miss = pc.getMiss(); + p2 = MProduct.get(Env.getCtx(), oak); + assertEquals(oak, p2.getM_Product_ID()); + assertTrue(pc.getMiss() > miss, "Get of product Oak after update of product Oak, cache miss should increase"); + + //cache for p1 not effected by p2 update, hit should increase + hit = pc.getHit(); + p1 = MProduct.get(Env.getCtx(), mulch); + assertEquals(mulch, p1.getM_Product_ID()); + assertTrue(pc.getHit() > hit, "Get of product Mulch after update of product Oak, cache hit should increase"); + + //create p3 to test delete + MProduct p3 = new MProduct(Env.getCtx(), 0, getTrxName()); + String name = "Test@"+System.currentTimeMillis(); + p3.setValue(name); + p3.setName(name); + p3.setM_Product_Category_ID(p1.getM_Product_Category_ID()); + p3.setC_UOM_ID(p1.getC_UOM_ID()); + p3.setC_TaxCategory_ID(p1.getC_TaxCategory_ID()); + p3.saveEx(); + + p3.deleteEx(true); + + //cache for p2 not effected by p3 delete, hit should increase + hit = pc.getHit(); + p2 = MProduct.get(Env.getCtx(), oak); + assertEquals(oak, p2.getM_Product_ID()); + assertTrue(pc.getHit() > hit, "Get of product Oak after delete of product Mulch, cache hit should increase"); + + //test update when cache is empty + CacheMgt.get().reset(); + p2.set_TrxName(getTrxName()); + p2.setDescription("Test1@"+System.currentTimeMillis()); + p2.saveEx(); + + rollback(); + } }