From 4790127ede4326bc6aa2098291823ca62c22b0e5 Mon Sep 17 00:00:00 2001 From: Alexandre Montplaisir Date: Thu, 23 Jun 2016 16:18:22 -0400 Subject: [PATCH] ss: Remove cache-level synchronization in HT_IO The LoadingCache supports multi-threaded access, we can remove the "synchronized" on the methods and only lock at the object level whenever we need to hit the disk. Benchmark results of the HistoryTreeBackendBenchmark comparing the current master version and the LoadingCache + this patch: (using System Time in all cases): Master LoadingCache + no-cache-synchro Average Build: 262ms 284ms Single Queries: 15ms 12ms Full Queries: 13ms 11ms Query History Range: 147ms 120ms Vertical scaling Build: 17.77s 19.82s Single Queries: 2.92s 3.2s Full Queries: 4.14s 4.42s Query History Range: 48.06s 57.03s Horizontal scaling Build: 10.54s 9.82s Single Queries: 186ms 110ms Full Queries: 157ms 86ms Query History Range: 14.52s 13.61s Interval distribution uniformly distributed Build: 428ms 402ms Single Queries: 31ms 24ms Full Queries: 26ms 16ms Query History Range: 368ms 443ms Interval durations with 10 percent outliers Build: 238ms 215ms Single Queries: 12ms 9ms Full Queries: 12ms 9ms Query History Range: 120ms 92ms Data type strings Build: 302ms 288ms Single Queries: 36ms 29ms Full Queries: 27ms 8ms Query History Range: 333ms 107ms Data type longs Build: 256ms 250ms Single Queries: 16ms 13ms Full Queries: 13ms 8ms Query History Range: 151ms 91ms Data type doubles Build: 264ms 248ms Single Queries: 18ms 14ms Full Queries: 14ms 7ms Query History Range: 161ms 90ms The "vertical scaling" seem strange, but remember this is a degenerate case where the average node usage is rounded to 0%. The LoadingCache seems slightly less resilient to cache thrashing, (which makes sense for LRU vs. direct-map in general) but it's faster in all other regular use cases. Other patches are being worked on to avoid trees with very low node usage. Change-Id: I96e32d99febb1dbbe7f9d2b473d15307c4b8e2e5 Signed-off-by: Alexandre Montplaisir Reviewed-on: https://git.eclipse.org/r/75886 Reviewed-by: Hudson CI Reviewed-by: Genevieve Bastien Tested-by: Genevieve Bastien --- .../core/backend/historytree/HT_IO.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) 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 37dfd2c005..ef04a424e4 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 @@ -98,8 +98,10 @@ class HT_IO { LOGGER.finest(() -> "[HtIo:CacheMiss] seqNum=" + seqNb); //$NON-NLS-1$ - io.seekFCToNodePos(io.fFileChannelIn, seqNb); - return HTNode.readNode(io.fConfig, io.fFileChannelIn); + synchronized (io) { + io.seekFCToNodePos(io.fFileChannelIn, seqNb); + return HTNode.readNode(io.fConfig, io.fFileChannelIn); + } } })); @@ -175,7 +177,7 @@ class HT_IO { * reading. Instead of using a big reader-writer lock, we'll * just catch this exception. */ - public synchronized @NonNull HTNode readNode(int seqNumber) throws ClosedChannelException { + public @NonNull HTNode readNode(int seqNumber) throws ClosedChannelException { /* 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); @@ -196,7 +198,7 @@ class HT_IO { } } - public synchronized void writeNode(HTNode node) { + public void writeNode(HTNode node) { try { int seqNumber = node.getSequenceNumber(); @@ -205,8 +207,10 @@ class HT_IO { NODE_CACHE.put(key, node); /* Position ourselves at the start of the node and write it */ - seekFCToNodePos(fFileChannelOut, seqNumber); - node.writeSelf(fFileChannelOut); + synchronized (this) { + seekFCToNodePos(fFileChannelOut, seqNumber); + node.writeSelf(fFileChannelOut); + } } catch (IOException e) { /* If we were able to open the file, we should be fine now... */ Activator.getDefault().logError(e.getMessage(), e); -- 2.34.1