From 51f45bdf95551409666a04f92355638f83c018da Mon Sep 17 00:00:00 2001 From: Tuukka Lehtonen Date: Fri, 13 Apr 2018 00:43:36 +0300 Subject: [PATCH] Fixed ClusterTable cluster hash/importance map book-keeping problem The main problem was that importance values should have been started from 1 and 0 is considered an invalid importance value. This prevents ClusterTable.replaceCluster from making mistakes in importanceMap. refs #7724 Change-Id: Ie15a2d11bef2b8bdbcee2f5baba47633faa97826 (cherry picked from commit f59872e129c2d1b84fe322ae120fbeac22a94f88) --- .../procore/internal/ClusterTable.java | 83 +++++++++++++++---- .../db/procore/cluster/ClusterSmall.java | 16 ++-- 2 files changed, 75 insertions(+), 24 deletions(-) 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 2c65ee8e8..5daf534fe 100644 --- 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.concurrent.atomic.AtomicLong; import java.util.function.Consumer; import org.simantics.databoard.Bindings; @@ -65,7 +66,7 @@ public final class ClusterTable implements IClusterTable { int maximumBytes = 128 * 1024 * 1024; int limit = (int)(0.8*(double)maximumBytes); - long timeCounter = 0; + private final AtomicLong timeCounter = new AtomicLong(1); final private SessionImplSocket sessionImpl; final private ArrayList writeOnlyClusters = new ArrayList(); @@ -93,6 +94,10 @@ public final class ClusterTable implements IClusterTable { public long getClusterId() { return clusterId; } + @Override + public String toString() { + return "CID " + clusterId; + } } private class Clusters { // This makes sure that non-null values from hashMap.get can be trusted (no unsynchronized rehashes occur) @@ -271,27 +276,35 @@ public final class ClusterTable implements IClusterTable { writeOnlyClusters.add(proxy); return clusters.create(proxy); } else { + //printMaps("makeCluster"); ClusterImpl cluster = ClusterImpl.make(clusterUID, clusterKey, sessionImpl.clusterTranslator); clusters.create(cluster); if (!cluster.isLoaded()) Logger.defaultLogError(new Exception("Bug in ClusterTable.makeCluster(long, boolean), cluster not loaded")); importanceMap.put(cluster.getImportance(), new ImportanceEntry(cluster)); + if (VALIDATE_SIZE) + validateSize("makeCluster"); if(collectorPolicy != null) collectorPolicy.added(cluster); return cluster; } } synchronized void replaceCluster(ClusterI cluster) { + //printMaps("replaceCluster"); checkCollect(); int clusterKey = cluster.getClusterKey(); ClusterI existing = clusterArray[clusterKey]; if (existing.hasVirtual()) cluster.markVirtual(); - importanceMap.remove(existing.getImportance()); + long existingImportance = existing.getImportance(); + if (existingImportance != 0) + importanceMap.remove(existingImportance); if (collectorPolicy != null) collectorPolicy.removed((ClusterImpl)existing); + //System.out.println("ClusterTable.replace(" + existing + " (I=" + existing.getImportance() + ") => " + cluster + " (I=" + cluster.getImportance() + ")"); + clusters.replace((ClusterImpl)cluster); if (!cluster.isLoaded()) Logger.defaultLogError(new Exception("Bug in ClusterTable.replaceCluster(ClusterI), cluster not loaded")); @@ -306,7 +319,8 @@ public final class ClusterTable implements IClusterTable { // This will update sizeInBytes through adjustCachedSize cluster.getCachedSize(); } - validateSize(); + if (VALIDATE_SIZE) + validateSize("replaceCluster"); } synchronized void release(CollectorCluster cluster) { @@ -322,6 +336,7 @@ public final class ClusterTable implements IClusterTable { return; if(!clusterImpl.isLoaded() || clusterImpl.isEmpty()) return; + //printMaps("release"); clusters.freeProxy(clusterImpl); importanceMap.remove(clusterImpl.getImportance()); if (collectorPolicy != null) @@ -331,6 +346,8 @@ public final class ClusterTable implements IClusterTable { if (!dirtySizeInBytes) { adjustCachedSize(-clusterImpl.getCachedSize(), clusterImpl); } + if (VALIDATE_SIZE) + validateSize("release"); } synchronized void compact(long id) { @@ -687,6 +704,7 @@ public final class ClusterTable implements IClusterTable { ClientChangesImpl cs = new ClientChangesImpl(session); if (session.clientChanges == null) session.clientChanges = cs; + //printMaps("refresh"); for (int i=0; i { + System.out.format("\t%d: %s%n", importance, ie.toString()); + }); + System.out.println("clusters.hashMap (" + ihms + "/" + clusters.hashMap.size() + "):"); clusters.hashMap.forEachEntry((cid, c) -> { - if (c != null && !c.isWriteOnly() && !c.isEmpty() && c.isLoaded()) { - //System.out.println(cid + ": " + c); - hms[0]++; - } + boolean important = importantCluster(c); + boolean wo = c != null && c.isWriteOnly(); + boolean empty = c != null && c.isEmpty(); + boolean loaded = c != null && c.isLoaded(); + System.out.format("\t%s: %d - %s (writeOnly=%b, empty=%b, loaded=%b)%n", important ? " I" : "NI", cid, c, wo, empty, loaded); 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)); - } + } + + private int countImportantClusters() { + int[] result = { 0 }; + clusters.hashMap.forEachEntry((cid, c) -> { + if (importantCluster(c)) + result[0]++; + return true; + }); + return result[0]; + } + + private static boolean importantCluster(ClusterImpl c) { + return c != null && !c.isWriteOnly() && c.isLoaded();// && !c.isEmpty(); } } diff --git a/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterSmall.java b/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterSmall.java index 1b7dc0d32..1a8af5143 100644 --- a/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterSmall.java +++ b/bundles/org.simantics.db.procore/src/org/simantics/db/procore/cluster/ClusterSmall.java @@ -1069,14 +1069,14 @@ final public class ClusterSmall extends ClusterImpl { public String toString() { if (deleted) return "ClusterSmall[" + getClusterId() + " - has been deleted or hasn't been created.]"; try { - final TIntHashSet set = new TIntHashSet(); - TIntShortHashMap map = foreignTable.getResourceHashMap(); - map.forEachKey(new TIntProcedure() { - @Override - public boolean execute(int value) { - set.add(value & 0xfffff000); - return true; - } + ForeignTableSmall ft = foreignTable; + if (ft == null) + return "ClusterSmall[" + getClusterId() + " - " + getNumberOfResources() + "]"; + TIntShortHashMap map = ft.getResourceHashMap(); + TIntHashSet set = new TIntHashSet(); + map.forEachKey(value -> { + set.add(value & 0xfffff000); + return true; }); return "ClusterSmall[" + getClusterId() + " - " + getNumberOfResources() + " - " + foreignTable.getResourceHashMap().size() + " - " + set.size() + "]"; } catch (Throwable e) { -- 2.47.1