segmentstore: fix incorrect iteration order in segment history

The initial values for minEnd and maxStart times would lead to
incorrect orders during tree build.
Also, the bounds for the node are now serialized.
Add a test to ensure the order is right.

Change-Id: I0c0811f725fbdb3c06e45685f511fac060876b6d
Signed-off-by: Loïc Prieur-Drevon <loic.prieurdrevon@gmail.com>
Reviewed-on: https://git.eclipse.org/r/95945
Tested-by: Genevieve Bastien <gbastien+lttng@versatic.net>
Reviewed-by: Hudson CI
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-by: Jean-Christian Kouame <jean-christian.kouame@ericsson.com>
Reviewed-by: Genevieve Bastien <gbastien+lttng@versatic.net>

diff --git a/analysis/org.eclipse.tracecompass.analysis.timing.ui.swtbot.tests/src/org/eclipse/tracecompass/analysis/timing/ui/swtbot/tests/table/SegmentTableTest.java b/analysis/org.eclipse.tracecompass.analysis.timing.ui.swtbot.tests/src/org/eclipse/tracecompass/analysis/timing/ui/swtbot/tests/table/SegmentTableTest.java
index 73ec2d5499c5fba1943c73a31e2ccc271486e922..be43cabc1db5a44710db8323293ef72b9f0307bc 100644 (file)
--- a/analysis/org.eclipse.tracecompass.analysis.timing.ui.swtbot.tests/src/org/eclipse/tracecompass/analysis/timing/ui/swtbot/tests/table/SegmentTableTest.java
+++ b/analysis/org.eclipse.tracecompass.analysis.timing.ui.swtbot.tests/src/org/eclipse/tracecompass/analysis/timing/ui/swtbot/tests/table/SegmentTableTest.java
@@ -414,7 +414,7 @@ public class SegmentTableTest {
             fBot.waitUntil(ConditionHelpers.isTableCellFilled(tableBot, "0", 0, 2));
             tableBot.header("Duration").click();
             // FIXME: Should be 999,999, but sorting on disk does not work well yet
-            fBot.waitUntil(ConditionHelpers.isTableCellFilled(tableBot, "818,799", 0, 2));
+            fBot.waitUntil(ConditionHelpers.isTableCellFilled(tableBot, "818,399", 0, 2));
         } finally {
             Files.delete(segmentFile);
         }

diff --git a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/perf/org/eclipse/tracecompass/segmentstore/core/tests/perf/SegmentStoreBenchmark.java b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/perf/org/eclipse/tracecompass/segmentstore/core/tests/perf/SegmentStoreBenchmark.java
index 6930b9d4093c04b5c153243700f4eb52b66f7d6f..5892d0fb9315b5a10bc77c37ad45d4b979ea5490 100644 (file)
--- a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/perf/org/eclipse/tracecompass/segmentstore/core/tests/perf/SegmentStoreBenchmark.java
+++ b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/perf/org/eclipse/tracecompass/segmentstore/core/tests/perf/SegmentStoreBenchmark.java
@@ -10,6 +10,7 @@
 package org.eclipse.tracecompass.segmentstore.core.tests.perf;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -188,6 +189,17 @@ public class SegmentStoreBenchmark {
             populate(size, fuzz, fSegStore, 0, getSegmentStoreSize());
             pMinsertion.stop();
 
+            if (i == 0) {
+                /**
+                 * Assert that the segments are returned in the correct order,
+                 * the benchmark will be irrelevant if the contract is not
+                 * respected.
+                 */
+                assertOrder(fSegStore, SegmentComparators.INTERVAL_START_COMPARATOR);
+                assertOrder(fSegStore, SegmentComparators.INTERVAL_END_COMPARATOR);
+                assertOrder(fSegStore, SegmentComparators.INTERVAL_LENGTH_COMPARATOR);
+            }
+
             pMiterateStart.start();
             int count = sortedIterate(fSegStore, SegmentComparators.INTERVAL_START_COMPARATOR);
             pMiterateStart.stop();
@@ -259,6 +271,19 @@ public class SegmentStoreBenchmark {
         return iterate(iterable);
     }
 
+    private static void assertOrder(ISegmentStore<@NonNull BasicSegment> store, Comparator<@NonNull ISegment> order) {
+        Iterable<@NonNull BasicSegment> iterable = store.iterator(order);
+        BasicSegment prev = null;
+        long count = 0L;
+        for (BasicSegment segment : iterable) {
+            if (prev != null) {
+                assertTrue("Incorrect iteration order at: " + count + ", prev: " + prev + ", current: " + segment, order.compare(prev, segment) <= 0);
+            }
+            prev = segment;
+            count++;
+        }
+    }
+
     private static void populate(int size, int[] fuzz, ISegmentStore<@NonNull BasicSegment> store, long low, long high) {
         for (long i = low; i < high; i++) {
             long start = i + fuzz[(int) (i % size)];

diff --git a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeCoreNodeTest.java b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeCoreNodeTest.java
index 9c7e4a360d7fdd68604ef995564c3e14a2c0ce6b..a541c2a28c147df0a9dd01bf092025f5d7d3c399 100644 (file)
--- a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeCoreNodeTest.java
+++ b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeCoreNodeTest.java
@@ -78,7 +78,7 @@ public class SegmentTreeCoreNodeTest extends HTCoreNodeTest<BasicSegment, Segmen
     public static Iterable<Object[]> getParameters() {
         return Arrays.asList(new Object[][] {
                 { "Segment tree core node",
-                    HTNode.COMMON_HEADER_SIZE + Integer.BYTES + Integer.BYTES * NB_CHILDREN + 6 * Long.BYTES * NB_CHILDREN,
+                    HTNode.COMMON_HEADER_SIZE + Integer.BYTES + Integer.BYTES * NB_CHILDREN + 6 * Long.BYTES * NB_CHILDREN + 4 * Long.BYTES,
                     SegmentTreeNodeStub.NODE_FACTORY,
                     BasicSegment.BASIC_SEGMENT_READ_FACTORY,
                     BASE_SEGMENT_FACTORY },
@@ -97,7 +97,7 @@ public class SegmentTreeCoreNodeTest extends HTCoreNodeTest<BasicSegment, Segmen
 
         // Verify the default values
         assertEquals(start, stub.getMaxStart());
-        assertEquals(Long.MAX_VALUE, stub.getMinEnd());
+        assertEquals(start, stub.getMinEnd());
         assertEquals(Long.MAX_VALUE, stub.getShortest());
         assertEquals(0, stub.getLongest());
 
@@ -137,7 +137,7 @@ public class SegmentTreeCoreNodeTest extends HTCoreNodeTest<BasicSegment, Segmen
 
         // Verify the default values
         assertEquals(start, parentStub.getMaxStart(0));
-        assertEquals(Long.MAX_VALUE, parentStub.getMinEnd(0));
+        assertEquals(start, parentStub.getMinEnd(0));
         assertEquals(Long.MAX_VALUE, parentStub.getShortest(0));
         assertEquals(0, parentStub.getLongest(0));
 
@@ -162,7 +162,7 @@ public class SegmentTreeCoreNodeTest extends HTCoreNodeTest<BasicSegment, Segmen
 
         // ... but not the parent's own data
         assertEquals(start, parentStub.getMaxStart());
-        assertEquals(Long.MAX_VALUE, parentStub.getMinEnd());
+        assertEquals(start, parentStub.getMinEnd());
         assertEquals(Long.MAX_VALUE, parentStub.getShortest());
         assertEquals(0, parentStub.getLongest());
     }

diff --git a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeNodeTest.java b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeNodeTest.java
index afaebcc98e93beed1a0738d9836b8deae593fb7b..c0c28b3f04209ecb80f3c7c8f6c31d1cd7d2c0fc 100644 (file)
--- a/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeNodeTest.java
+++ b/statesystem/org.eclipse.tracecompass.segmentstore.core.tests/src/org/eclipse/tracecompass/segmentstore/core/tests/htStore/SegmentTreeNodeTest.java
@@ -69,7 +69,7 @@ public class SegmentTreeNodeTest extends HTNodeTest<BasicSegment, SegmentTreeNod
     public static Iterable<Object[]> getParameters() {
         return Arrays.asList(new Object[][] {
                 { "Segment tree node",
-                        HTNode.COMMON_HEADER_SIZE,
+                        HTNode.COMMON_HEADER_SIZE + 4 * Long.BYTES,
                         SegmentTreeNodeStub.NODE_FACTORY,
                         BasicSegment.BASIC_SEGMENT_READ_FACTORY,
                         BASE_SEGMENT_FACTORY

diff --git a/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/internal/segmentstore/core/segmentHistoryTree/SegmentTreeNode.java b/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/internal/segmentstore/core/segmentHistoryTree/SegmentTreeNode.java
index 9c3e80e20995b30596b1bc08549b9f7e30e34e5a..950068ee67c85744de52898ab2871691ead896d2 100644 (file)
--- a/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/internal/segmentstore/core/segmentHistoryTree/SegmentTreeNode.java
+++ b/statesystem/org.eclipse.tracecompass.segmentstore.core/src/org/eclipse/tracecompass/internal/segmentstore/core/segmentHistoryTree/SegmentTreeNode.java
@@ -363,7 +363,7 @@ public class SegmentTreeNode<E extends ISegment> extends OverlappingNode<E> {
      * @return the earliest end time of this node's intervals
      */
     public long getMinEnd() {
-        return fMinEnd;
+        return fMinEnd != Long.MAX_VALUE ? fMinEnd : getNodeStart();
     }
 
     /**
@@ -384,6 +384,29 @@ public class SegmentTreeNode<E extends ISegment> extends OverlappingNode<E> {
         return fLongest;
     }
 
+    @Override
+    protected void readSpecificHeader(@NonNull ByteBuffer buffer) {
+        super.readSpecificHeader(buffer);
+        fMaxStart = buffer.getLong();
+        fMinEnd = buffer.getLong();
+        fShortest = buffer.getLong();
+        fLongest = buffer.getLong();
+    }
+
+    @Override
+    protected void writeSpecificHeader(@NonNull ByteBuffer buffer) {
+        super.writeSpecificHeader(buffer);
+        buffer.putLong(fMaxStart);
+        buffer.putLong(fMinEnd);
+        buffer.putLong(fShortest);
+        buffer.putLong(fLongest);
+    }
+
+    @Override
+    protected int getSpecificHeaderSize() {
+        return super.getSpecificHeaderSize() + 4 * Long.BYTES;
+    }
+
     /**
      * Get the children intersecting the given time range, and return the least
      * element for each child wrt to the comparator