diff --git a/api/src/org/labkey/api/cache/CacheWrapper.java b/api/src/org/labkey/api/cache/CacheWrapper.java index 2a30dc4ba2b..0e820278edb 100644 --- a/api/src/org/labkey/api/cache/CacheWrapper.java +++ b/api/src/org/labkey/api/cache/CacheWrapper.java @@ -20,6 +20,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.mbean.CacheMXBean; import org.labkey.api.util.Filter; +import org.labkey.api.util.HeartBeat; import org.labkey.api.view.ViewServlet; import javax.management.DynamicMBean; @@ -44,6 +45,11 @@ class CacheWrapper implements TrackingCache, CacheMXBean private final @Nullable StackTraceElement[] _stackTrace; private final V _nullMarker = (V)NULL_MARKER; + // Issue 51702. Calculating the size of large caches can be expensive. It's OK to be a little stale or + // miss a brief spike by only updating the max size stat every so often + private boolean _maxSizeDirty; + private long _maxSizeNextUpdate; + CacheWrapper(@NotNull SimpleCache cache, @NotNull String debugName, @Nullable Stats stats, @Nullable StackTraceElement[] stackTrace) { _cache = cache; @@ -212,6 +218,7 @@ public String getDebugName() @Override public Stats getStats() { + updateMaxSize(); return _stats; } @@ -248,6 +255,7 @@ public String toString() private V trackGet(V value) { + updateMaxSizeIfStale(); _stats.gets.incrementAndGet(); if (value == null) @@ -263,25 +271,47 @@ private void trackPut(V value) _stats.puts.incrementAndGet(); + _maxSizeDirty = true; + updateMaxSizeIfStale(); + } + + /** @return current size */ + private int updateMaxSize() + { + _maxSizeDirty = false; + // Don't update the max size more than once every five seconds + _maxSizeNextUpdate = HeartBeat.currentTimeMillis() + 5_000; long maxSize = _stats.max_size.get(); - long currentSize = size(); + int currentSize = size(); if (currentSize > maxSize) _stats.max_size.compareAndSet(maxSize, currentSize); + return currentSize; + } + + private void updateMaxSizeIfStale() + { + if (_maxSizeDirty && _maxSizeNextUpdate <= HeartBeat.currentTimeMillis()) + { + updateMaxSize(); + } } private void trackRemove() { + updateMaxSizeIfStale(); _stats.removes.incrementAndGet(); } private int trackRemoves(int removes) { + updateMaxSizeIfStale(); _stats.removes.addAndGet(removes); return removes; } private void trackClear() { + updateMaxSizeIfStale(); _stats.clears.incrementAndGet(); } @@ -296,7 +326,8 @@ public SimpleCache getWrappedCache() @Override public int getSize() { - return size(); + // Since we're being asked for the current size, go ahead and update the max size stat too + return updateMaxSize(); } @Override diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index b22d8744cdf..96f7a59c0e4 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -360,7 +360,7 @@ public static Container createContainer(Container parent, String name, @Nullable SecurityManager.setAdminOnlyPermissions(c, savePolicyUser); } - _removeFromCache(c); // seems odd, but it removes c.getProject() which clears other things from the cache + _removeFromCache(c, true); // seems odd, but it removes c.getProject() which clears other things from the cache // Initialize the list of active modules in the Container c.getActiveModules(true, true, user); @@ -382,7 +382,7 @@ public static Container createContainer(Container parent, String name, @Nullable // NOTE parent caches some info about children (e.g. hasWorkbookChildren) // since mutating cached objects is frowned upon, just uncache parent // CONSIDER: we could perhaps only uncache if the child is a workbook, but I think this reasonable - _removeFromCache(parent); + _removeFromCache(parent, true); if (null != configureContainer) configureContainer.accept(c); @@ -511,7 +511,7 @@ public static void setFolderType(Container c, FolderType folderType, User user, folderType.configureContainer(c, user); // Configure new only after folder type has been changed // TODO: Not needed? I don't think we've changed the container's state. - _removeFromCache(c); + _removeFromCache(c, false); } else { @@ -810,7 +810,7 @@ public static void updateDescription(Container container, String description, Us new SqlExecutor(CORE.getSchema()).execute(sql, description, container.getRowId()); String oldValue = container.getDescription(); - _removeFromCache(container); + _removeFromCache(container, false); container = getForRowId(container.getRowId()); ContainerPropertyChangeEvent evt = new ContainerPropertyChangeEvent(container, user, Property.Description, oldValue, description); firePropertyChangeEvent(evt); @@ -825,7 +825,7 @@ public static void updateSearchable(Container container, boolean searchable, Use sql.append(" SET Searchable=? WHERE RowID=?"); new SqlExecutor(CORE.getSchema()).execute(sql, searchable, container.getRowId()); - _removeFromCache(container); + _removeFromCache(container, false); } public static void updateLockState(Container container, LockState lockState, @NotNull Runnable auditRunnable) @@ -837,7 +837,7 @@ public static void updateLockState(Container container, LockState lockState, @No sql.append(" SET LockState = ?, ExpirationDate = NULL WHERE RowID = ?"); new SqlExecutor(CORE.getSchema()).execute(sql, lockState, container.getRowId()); - _removeFromCache(container); + _removeFromCache(container, false); auditRunnable.run(); } @@ -920,7 +920,7 @@ public static void updateExpirationDate(Container container, LocalDate expiratio // Note: jTDS doesn't support LocalDate, so convert to java.sql.Date new SqlExecutor(CORE.getSchema()).execute(sql, java.sql.Date.valueOf(expirationDate), container.getRowId()); - _removeFromCache(container); + _removeFromCache(container, false); auditRunnable.run(); } @@ -934,7 +934,7 @@ public static void updateType(Container container, String newType, User user) sql.append(" SET Type=? WHERE RowID=?"); new SqlExecutor(CORE.getSchema()).execute(sql, newType, container.getRowId()); - _removeFromCache(container); + _removeFromCache(container, false); } public static void updateTitle(Container container, String title, User user) @@ -949,7 +949,7 @@ public static void updateTitle(Container container, String title, User user) sql.append(" SET Title=? WHERE RowID=?"); new SqlExecutor(CORE.getSchema()).execute(sql, title, container.getRowId()); - _removeFromCache(container); + _removeFromCache(container, false); String oldValue = container.getTitle(); container = getForRowId(container.getRowId()); ContainerPropertyChangeEvent evt = new ContainerPropertyChangeEvent(container, user, Property.Title, oldValue, title); @@ -958,7 +958,7 @@ public static void updateTitle(Container container, String title, User user) public static void uncache(Container c) { - _removeFromCache(c); + _removeFromCache(c, true); } public static final String SHARED_CONTAINER_PATH = "/Shared"; @@ -1910,7 +1910,7 @@ private static boolean delete(final Container c, User user, @Nullable String com if (sel.exists()) { - _removeFromCache(c); + _removeFromCache(c, true); return false; } @@ -1948,7 +1948,7 @@ private static boolean delete(final Container c, User user, @Nullable String com DATABASE_QUERY_LOCK.lock(); try { - _removeFromCache(c); + _removeFromCache(c, true); } finally { @@ -2097,15 +2097,23 @@ private static void _clearChildrenFromCache(Container c) navTreeManageUncache(c); } - private static void _removeFromCache(Container c) + /** @param hierarchyChange whether the shape of the container tree has changed */ + private static void _removeFromCache(Container c, boolean hierarchyChange) { CACHE.remove(toString(c)); CACHE.remove(c.getId()); - // blow away the children caches - CACHE_CHILDREN.clear(); + // Blow away the children caches, which can be outdated even if the hierarchy hasn't changed + // For example, changing the folder type or enabled modules in a parent can impact child workbooks CACHE_ALL_CHILDREN.clear(); + if (hierarchyChange) + { + // This is strictly keeping track of the parent/child relationships themselves so it only needs to be + // cleared when the tree changes + CACHE_CHILDREN.clear(); + } + navTreeManageUncache(c); } @@ -2146,7 +2154,7 @@ public static void notifyContainerChange(String id, Property prop, @Nullable Use Container c = getForId(id); if (null != c) { - _removeFromCache(c); + _removeFromCache(c, false); c = getForId(id); // load a fresh container since the original might be stale. if (null != c) {