Some fixes/cleanup for cluster table size caching logic.

The previous code was not keeping the cached size properly up-to-date
which resulted in the DB client thinking that the consumer cluster
memory size is larger than it actually is and eventually winding up in a
state where the LRU importanceMap is constantly almost empty and the
code keeps throwing away clusters whenever the next cluster is loaded
due the misconception of used cluster memory.

The code is still not perfect/totally functional - I was still able to
get the thrashing situation to reproduce with A6 model imports, but not
as heavily as originally. I was able to import models with much more
initial conditions stored than previously without these changes.

refs #7598

Change-Id: I039fa2062908c05a61efb28e695daec01afd1725

diff --git a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterInfo.java b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterInfo.java
index 57dfe9f41442c0a327012e5f98efe02145bf2cba..f20b384569d2c9893e7a3999756cd3308409ab2b 100644 (file)
--- a/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterInfo.java
+++ b/bundles/org.simantics.acorn/src/org/simantics/acorn/lru/ClusterInfo.java
@@ -245,11 +245,9 @@ public class ClusterInfo extends LRUObject<ClusterUID, ClusterInfo> implements P
        }
        
        public ClusterImpl getForUpdate() throws SDBException {
+               acquireMutex();
                try {
-                       acquireMutex();
-
                        assert(updateState != null);
-                       
                        makeResident();
                        setDirty(true);
                        updateState.beginUpdate();

diff --git a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/util/SessionGarbageCollection.java b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/util/SessionGarbageCollection.java
index 26e049db67d5d95c87e47e56d81b9ad843ac4572..c168e22860b25dba84ff1b715376c4a878effcfb 100644 (file)
--- a/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/util/SessionGarbageCollection.java
+++ b/bundles/org.simantics.db.layer0/src/org/simantics/db/layer0/util/SessionGarbageCollection.java
@@ -25,7 +25,6 @@ import org.simantics.db.service.ClusterControl;
 import org.simantics.db.service.LifecycleSupport;
 import org.simantics.db.service.QueryControl;
 import org.simantics.utils.DataContainer;
-import org.simantics.utils.datastructures.Callback;
 
 /**
  * @author Tuukka Lehtonen
@@ -132,15 +131,12 @@ public class SessionGarbageCollection {
                     Logger.defaultLogError(e);
             }
         } else {
-            session.asyncRequest(request, new Callback<DatabaseException>() {
-                @Override
-                public void run(DatabaseException e) {
-                    if (e != null) {
-                        if (errorCallback != null)
-                            errorCallback.accept(e);
-                        else
-                            Logger.defaultLogError(e);
-                    }
+            session.asyncRequest(request, e -> {
+                if (e != null) {
+                    if (errorCallback != null)
+                        errorCallback.accept(e);
+                    else
+                        Logger.defaultLogError(e);
                 }
             });
         }

diff --git a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterTable.java b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterTable.java
index 123f346a4dd0322254aa74bfd1a259b9bd07357c..99aa9004f34d610f7a66636a47a9659d254566ee 100644 (file)
--- a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterTable.java
+++ b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/ClusterTable.java
@@ -18,6 +18,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.concurrent.Semaphore;
+import java.util.function.Consumer;
 
 import org.simantics.databoard.Bindings;
 import org.simantics.db.ClusterCreator;
@@ -46,7 +47,6 @@ import org.simantics.db.service.ClusterCollectorPolicy;
 import org.simantics.db.service.ClusterCollectorPolicy.CollectorCluster;
 import org.simantics.db.service.ClusterUID;
 import org.simantics.utils.Development;
-import org.simantics.utils.datastructures.Callback;
 
 import fi.vtt.simantics.procore.DebugPolicy;
 import fi.vtt.simantics.procore.internal.ClusterControlImpl.ClusterStateImpl;
@@ -60,6 +60,8 @@ import gnu.trove.set.hash.TIntHashSet;
 
 public final class ClusterTable implements IClusterTable {
 
+    private static final boolean VALIDATE_SIZE = false;
+
     int maximumBytes = 128 * 1024 * 1024;
     int limit = (int)(0.8*(double)maximumBytes);
 
@@ -297,10 +299,14 @@ public final class ClusterTable implements IClusterTable {
         importanceMap.put(cluster.getImportance(), new ImportanceEntry((ClusterImpl)cluster));
         if(collectorPolicy != null) collectorPolicy.added((ClusterImpl)cluster);
 
-        if (!dirtySizeInBytes && !existing.isLoaded()) {
-            sizeInBytes += cluster.getCachedSize();
+        if (!dirtySizeInBytes) {
+            if (existing != cluster) {
+                adjustCachedSize(-existing.getCachedSize(), existing);
+            }
+            // This will update sizeInBytes through adjustCachedSize
+            cluster.getCachedSize();
         }
-
+        validateSize();
     }
 
     synchronized void release(CollectorCluster cluster) {
@@ -309,6 +315,8 @@ public final class ClusterTable implements IClusterTable {
     }
 
     synchronized void release(long clusterId) {
+        //System.out.println("ClusterTable.release(" + clusterId + "): " + sizeInBytes);
+        //validateSize();
         ClusterImpl clusterImpl = clusters.getClusterByClusterId(clusterId);
         if (null == clusterImpl)
             return;
@@ -321,7 +329,7 @@ public final class ClusterTable implements IClusterTable {
         if (sessionImpl.writeState != null)
             sessionImpl.clusterStream.flush(clusterImpl.clusterUID);
         if (!dirtySizeInBytes) {
-            sizeInBytes -= clusterImpl.getCachedSize();
+            adjustCachedSize(-clusterImpl.getCachedSize(), clusterImpl);
         }
     }
 
@@ -941,12 +949,9 @@ public final class ClusterTable implements IClusterTable {
         final Semaphore s = new Semaphore(0);
         final DatabaseException[] ex = new DatabaseException[1];
 
-        load2(clusterId, clusterKey, new Callback<DatabaseException>() {
-            @Override
-            public void run(DatabaseException e) {
-                ex[0] = e;
-                s.release();
-            }
+        load2(clusterId, clusterKey, e -> {
+            ex[0] = e;
+            s.release();
         });
         try {
             s.acquire();
@@ -1019,7 +1024,7 @@ public final class ClusterTable implements IClusterTable {
         }
     }
 
-    public synchronized void load2(long clusterId, int clusterKey, final Callback<DatabaseException> runnable) {
+    public synchronized void load2(long clusterId, int clusterKey, final Consumer<DatabaseException> runnable) {
 
         assert (Constants.ReservedClusterId != clusterId);
 
@@ -1061,7 +1066,7 @@ public final class ClusterTable implements IClusterTable {
                 e = new DatabaseException("Load cluster failed.", t);
         }
         if (null == cluster) {
-            runnable.run(e);
+            runnable.accept(e);
             return;
         }
 
@@ -1070,7 +1075,7 @@ public final class ClusterTable implements IClusterTable {
         // Can not be called with null argument.
         replaceCluster(cluster);
         sessionImpl.onClusterLoaded(clusterId);
-        runnable.run(null);
+        runnable.accept(null);
 
     }
 
@@ -1162,4 +1167,47 @@ public final class ClusterTable implements IClusterTable {
         return getClusterKeyByClusterUIDOrMakeProxy(ClusterUID.make(first, second));
     }
 
+    public void adjustCachedSize(long l, ClusterI cluster) {
+        if (l != 0) {
+            //System.out.println("ClusterTable: adjusting cluster table cached size by " + l + ": " + sizeInBytes + " -> "
+            //        + (sizeInBytes + l) + ", for cluster " + cluster.getClusterId() + " (" + cluster + ")");
+            sizeInBytes += l;
+        }
+    }
+
+    private void validateSize() {
+        if (!VALIDATE_SIZE)
+            return;
+
+//        System.out.println("validating cached cluster sizes: " + sizeInBytes + ", hashMap.size="
+//                + clusters.hashMap.size() + ", importanceMap.size=" + importanceMap.size());
+
+        int i = clusterArray.length;
+        long size = 0;
+        for (int j = 0; j < i; ++j) {
+            ClusterI c = clusterArray[j];
+            if (c == null)
+                continue;
+            size += c.getCachedSize();
+        }
+        if (sizeInBytes != size) {
+            if (!dirtySizeInBytes)
+                System.out.println("BUG: CACHED CLUSTER SIZE DIFFERS FROM CALCULATED: " + sizeInBytes + " != " + size + ", delta = " + (sizeInBytes - size));
+            //else System.out.println("\"BUG?\": SIZES DIFFER: " + sizeInBytes + " != " + size + ", delta = " + (sizeInBytes - size));
+        }
+
+        int ims = importanceMap.size();
+        int[] hms = {0};
+        clusters.hashMap.forEachEntry((cid, c) -> {
+            if (c != null && !c.isWriteOnly() && !c.isEmpty() && c.isLoaded()) {
+                //System.out.println(cid + ": " + c);
+                hms[0]++;
+            }
+            return true;
+        });
+        if (Math.abs(ims-hms[0]) > 0) {
+            System.out.println("BUG2: hashmap and importanceMap sizes differ: " + hms[0] + " != " + ims + ", delta=" + (hms[0] - ims));
+        }
+    }
+
 }

diff --git a/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterImpl.java b/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterImpl.java
index 1baef88ac4fe9af7d9b664cef1b8026aff078f47..4c8e19853f6b15ba9de73e3f08581580c9a25f60 100644 (file)
--- a/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterImpl.java
+++ b/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterImpl.java
@@ -118,12 +118,17 @@ public abstract class ClusterImpl extends ClusterBase implements Modifier, Colle
     
     public long getCachedSize() {
         if(dirtySizeInBytes) {
+            long oldSize = sizeInBytes;
+            if (oldSize > 0)
+                clusterTable.adjustCachedSize(-oldSize, this);
             try {
                 sizeInBytes = getUsedSpace();
-                //System.err.println("recomputed size of cluster " + getClusterId() + " => " + sizeInBytes);
+                //System.out.println("recomputed size of cluster " + getClusterId() + ": " + oldSize + " => " + sizeInBytes);
             } catch (DatabaseException e) {
                 Logger.defaultLogError(e);
             }
+            if (sizeInBytes != 0)
+                clusterTable.adjustCachedSize(sizeInBytes, this);
             dirtySizeInBytes = false;
         }
         return sizeInBytes;