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 <alexmonthy@efficios.com>
Reviewed-on: https://git.eclipse.org/r/75886
Reviewed-by: Hudson CI
Reviewed-by: Genevieve Bastien <gbastien+lttng@versatic.net>
Tested-by: Genevieve Bastien <gbastien+lttng@versatic.net>

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 37dfd2c0051e1a5450af0f75bc848d36add38e8d..ef04a424e4de3aff0c9be4eee2bf1f9e584f11d5 100644 (file)
--- 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);