From: Alexandre Montplaisir Date: Thu, 14 Apr 2016 21:23:19 +0000 (-0400) Subject: ss: Rework the HTNode cache X-Git-Url: http://git.efficios.com/?a=commitdiff_plain;h=1f48ef6615540ff27fe91c838a2f5ced5fecad38;p=deliverable%2Ftracecompass.git ss: Rework the HTNode cache Reimplement the HTNode cache as a Guava LoadingCache. This nicely abstracts the loading logic, and will also allow making cache accesses multi-threaded. This also changes the log points from CacheHit/CacheMiss to CacheLookup/CacheMiss, since the "cache hit" branch is now entirely part of the Guava library, so we cannot instrument it. Same information is available, it will just have to be computed differently. Change-Id: I267008c69f9d6f4ada0257dee45b2a79902b8c84 Signed-off-by: Alexandre Montplaisir Reviewed-on: https://git.eclipse.org/r/70713 Reviewed-by: Hudson CI Reviewed-by: Genevieve Bastien --- diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core/META-INF/MANIFEST.MF b/statesystem/org.eclipse.tracecompass.statesystem.core/META-INF/MANIFEST.MF index f5b6d398c3..e8f02aede8 100644 --- a/statesystem/org.eclipse.tracecompass.statesystem.core/META-INF/MANIFEST.MF +++ b/statesystem/org.eclipse.tracecompass.statesystem.core/META-INF/MANIFEST.MF @@ -23,4 +23,5 @@ Export-Package: org.eclipse.tracecompass.internal.provisional.statesystem.core.s org.eclipse.tracecompass.statesystem.core.statevalue Import-Package: com.google.common.annotations;version="15.0.0", com.google.common.base, + com.google.common.cache, com.google.common.collect;version="12.0.0" diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HT_IO.java b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HT_IO.java index 9b9823d54f..37dfd2c005 100644 --- a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HT_IO.java +++ b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HT_IO.java @@ -12,6 +12,8 @@ package org.eclipse.tracecompass.internal.statesystem.core.backend.historytree; +import static org.eclipse.tracecompass.common.core.NonNullUtils.checkNotNull; + import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -21,10 +23,17 @@ import java.nio.channels.FileChannel; import java.util.logging.Logger; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.tracecompass.common.core.log.TraceCompassLog; +import java.util.Objects; +import java.util.concurrent.ExecutionException; + +import org.eclipse.jdt.annotation.Nullable; import org.eclipse.tracecompass.internal.statesystem.core.Activator; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; + /** * This class abstracts inputs/outputs of the HistoryTree nodes. * @@ -38,25 +47,67 @@ import org.eclipse.tracecompass.internal.statesystem.core.Activator; */ class HT_IO { - @NonNullByDefault - private static final class CacheElement { - private final HTNode value; - private final HT_IO key; + private static final Logger LOGGER = TraceCompassLog.getLogger(HT_IO.class); + + // ------------------------------------------------------------------------ + // Global cache of nodes + // ------------------------------------------------------------------------ + + private static final class CacheKey { - public CacheElement(HT_IO ss, HTNode node) { - key = ss; - value = node; + public final HT_IO fStateHistory; + public final int fSeqNumber; + + public CacheKey(HT_IO stateHistory, int seqNumber) { + fStateHistory = stateHistory; + fSeqNumber = seqNumber; } - public HT_IO getKey() { - return key; + @Override + public int hashCode() { + return Objects.hash(fStateHistory, fSeqNumber); } - public HTNode getValue() { - return value; + @Override + public boolean equals(@Nullable Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + CacheKey other = (CacheKey) obj; + return (fStateHistory.equals(other.fStateHistory) && + fSeqNumber == other.fSeqNumber); } } + private static final int CACHE_SIZE = 256; + + private static final LoadingCache NODE_CACHE = + checkNotNull(CacheBuilder.newBuilder() + .maximumSize(CACHE_SIZE) + .build(new CacheLoader() { + @Override + public HTNode load(CacheKey key) throws IOException { + HT_IO io = key.fStateHistory; + int seqNb = key.fSeqNumber; + + LOGGER.finest(() -> "[HtIo:CacheMiss] seqNum=" + seqNb); //$NON-NLS-1$ + + io.seekFCToNodePos(io.fFileChannelIn, seqNb); + return HTNode.readNode(io.fConfig, io.fFileChannelIn); + } + })); + + + // ------------------------------------------------------------------------ + // Instance fields + // ------------------------------------------------------------------------ + /* Configuration of the History Tree */ private final HTConfig fConfig; @@ -66,15 +117,11 @@ class HT_IO { private final FileChannel fFileChannelIn; private final FileChannel fFileChannelOut; - // TODO test/benchmark optimal cache size - /** - * Cache size, must be a power of 2 - */ - private static final int CACHE_SIZE = 256; - private static final int CACHE_MASK = CACHE_SIZE - 1; - private static final CacheElement NODE_CACHE[] = new CacheElement[CACHE_SIZE]; + // ------------------------------------------------------------------------ + // Methods + // ------------------------------------------------------------------------ + - private static final Logger LOGGER = TraceCompassLog.getLogger(HT_IO.class); /** * Standard constructor @@ -129,30 +176,20 @@ class HT_IO { * just catch this exception. */ public synchronized @NonNull HTNode readNode(int seqNumber) throws ClosedChannelException { - /* Do a cache lookup */ - int offset = (seqNumber + hashCode()) & CACHE_MASK; - CacheElement cachedNode = NODE_CACHE[offset]; - - if (cachedNode != null && cachedNode.getKey() == this && cachedNode.getValue().getSequenceNumber() == seqNumber) { - LOGGER.finest(() -> "[HtIo:CacheHit] seqNum=" + seqNumber); //$NON-NLS-1$ - return cachedNode.getValue(); - } - - /* Lookup on disk */ + /* Do a cache lookup. If it's not present it will be loaded from disk */ + LOGGER.finest(() -> "[HtIo:CacheLookup] seqNum=" + seqNumber); //$NON-NLS-1$ + CacheKey key = new CacheKey(this, seqNumber); try { - seekFCToNodePos(fFileChannelIn, seqNumber); - HTNode readNode = HTNode.readNode(fConfig, fFileChannelIn); - LOGGER.finest(() -> "[HtIo:CacheMiss] seqNum=" + seqNumber); //$NON-NLS-1$ - - /* Put the node in the cache. */ - NODE_CACHE[offset] = new CacheElement(this, readNode); - return readNode; + return checkNotNull(NODE_CACHE.get(key)); - } catch (ClosedChannelException e) { - throw e; - } catch (IOException e) { + } catch (ExecutionException e) { + /* Get the inner exception that was generated */ + Throwable cause = e.getCause(); + if (cause instanceof ClosedChannelException) { + throw (ClosedChannelException) cause; + } /* - * Other types of IOExceptions shouldn't happen at this point though + * Other types of IOExceptions shouldn't happen at this point though. */ Activator.getDefault().logError(e.getMessage(), e); throw new IllegalStateException(); @@ -161,10 +198,11 @@ class HT_IO { public synchronized void writeNode(HTNode node) { try { - /* Insert the node into the cache. */ int seqNumber = node.getSequenceNumber(); - int offset = (seqNumber + hashCode()) & CACHE_MASK; - NODE_CACHE[offset] = new CacheElement(this, node); + + /* "Write-back" the node into the cache */ + CacheKey key = new CacheKey(this, seqNumber); + NODE_CACHE.put(key, node); /* Position ourselves at the start of the node and write it */ seekFCToNodePos(fFileChannelOut, seqNumber);