From: Matthew Khouzam Date: Tue, 15 Mar 2016 00:28:02 +0000 (-0400) Subject: ss: no longer have a strings section in the HTNodes X-Git-Url: http://git.efficios.com/?a=commitdiff_plain;h=59d30d83be5a8b926730d3107e0959fbbc7095c1;p=deliverable%2Ftracecompass.git ss: no longer have a strings section in the HTNodes The state systems nodes had a string section to allow for faster seeks on the state system file if the entire interval was not loaded to memory. This is never the case and it adds an extra integer to each non-integer state as well as extra (but well tested) logic. This patch makes the state sytem store each interval sequentially on a given node. Expect minimal performance difference, however, the size of the state system may shrink a bit (approx 5%). Change-Id: I01bc8594b7944fad97b6c9b715b848b6afa39913 Signed-off-by: Matthew Khouzam Signed-off-by: Alexandre Montplaisir Reviewed-on: https://git.eclipse.org/r/68404 Reviewed-by: Hudson CI --- diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core.tests/src/org/eclipse/tracecompass/statesystem/core/tests/backend/historytree/HistoryTreeTest.java b/statesystem/org.eclipse.tracecompass.statesystem.core.tests/src/org/eclipse/tracecompass/statesystem/core/tests/backend/historytree/HistoryTreeTest.java index 701913be41..c90f7142a1 100644 --- a/statesystem/org.eclipse.tracecompass.statesystem.core.tests/src/org/eclipse/tracecompass/statesystem/core/tests/backend/historytree/HistoryTreeTest.java +++ b/statesystem/org.eclipse.tracecompass.statesystem.core.tests/src/org/eclipse/tracecompass/statesystem/core/tests/backend/historytree/HistoryTreeTest.java @@ -36,18 +36,22 @@ import org.junit.Test; */ public class HistoryTreeTest { + /* Minimal allowed blocksize */ private static final int BLOCK_SIZE = HistoryTree.TREE_HEADER_SIZE; - /* The extra size used by long and double values */ - private static final int LONG_DOUBLE_SIZE = 8; - /* The number of extra characters to store a string interval */ - private static final int STRING_PADDING = 2; + + private static final HTInterval NULL_INTERVAL = new HTInterval(10, 20, 1, TmfStateValue.nullValue()); /* String with 23 characters, interval in file will be 25 bytes long */ private static final String TEST_STRING = "abcdefghifklmnopqrstuvw"; private static final TmfStateValue STRING_VALUE = TmfStateValue.newValueString(TEST_STRING); + private static final HTInterval STRING_INTERVAL = new HTInterval(10, 20, 1, STRING_VALUE); + private static final TmfStateValue LONG_VALUE = TmfStateValue.newValueLong(10L); + private static final HTInterval LONG_INTERVAL = new HTInterval(10, 20, 1, LONG_VALUE); + private static final TmfStateValue INT_VALUE = TmfStateValue.newValueInt(1); + private static final HTInterval INT_INTERVAL = new HTInterval(10, 20, 1, INT_VALUE); private @Nullable File fTempFile; @@ -132,8 +136,9 @@ public class HistoryTreeTest { /* Fill the following leaf node */ HTNode node = ht.getLatestLeaf(); + int intervalSize = STRING_INTERVAL.getSizeOnDisk(); int nodeFreeSpace = node.getNodeFreeSpace(); - int nbIntervals = nodeFreeSpace / (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING); + int nbIntervals = nodeFreeSpace / intervalSize; long ret = fillValues(ht, STRING_VALUE, nbIntervals, leafNodeStart); /* Make sure we haven't changed the depth or node count */ @@ -158,27 +163,31 @@ public class HistoryTreeTest { /* Add null intervals up to ~10% */ int nodeFreeSpace = node.getNodeFreeSpace(); - int nbIntervals = nodeFreeSpace / 10 / HTInterval.DATA_ENTRY_SIZE; + int intervalSize = NULL_INTERVAL.getSizeOnDisk(); + int nbIntervals = nodeFreeSpace / 10 / intervalSize; long start = fillValues(ht, TmfStateValue.nullValue(), nbIntervals, 1); - assertEquals(nodeFreeSpace - nbIntervals * HTInterval.DATA_ENTRY_SIZE, node.getNodeFreeSpace()); + assertEquals(nodeFreeSpace - nbIntervals * intervalSize , node.getNodeFreeSpace()); /* Add integer intervals up to ~20% */ nodeFreeSpace = node.getNodeFreeSpace(); - nbIntervals = nodeFreeSpace / 10 / HTInterval.DATA_ENTRY_SIZE; + intervalSize = INT_INTERVAL.getSizeOnDisk(); + nbIntervals = nodeFreeSpace / 10 / intervalSize; start = fillValues(ht, INT_VALUE, nbIntervals, start); - assertEquals(nodeFreeSpace - nbIntervals * HTInterval.DATA_ENTRY_SIZE, node.getNodeFreeSpace()); + assertEquals(nodeFreeSpace - nbIntervals * intervalSize , node.getNodeFreeSpace()); /* Add long intervals up to ~30% */ nodeFreeSpace = node.getNodeFreeSpace(); - nbIntervals = nodeFreeSpace / 10 / (HTInterval.DATA_ENTRY_SIZE + LONG_DOUBLE_SIZE); + intervalSize = LONG_INTERVAL.getSizeOnDisk(); + nbIntervals = nodeFreeSpace / 10 / intervalSize; start = fillValues(ht, LONG_VALUE, nbIntervals, start); - assertEquals(nodeFreeSpace - nbIntervals * (HTInterval.DATA_ENTRY_SIZE + LONG_DOUBLE_SIZE), node.getNodeFreeSpace()); + assertEquals(nodeFreeSpace - nbIntervals * intervalSize , node.getNodeFreeSpace()); /* Add string intervals up to ~40% */ nodeFreeSpace = node.getNodeFreeSpace(); - nbIntervals = nodeFreeSpace / 10 / (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING); + intervalSize = STRING_INTERVAL.getSizeOnDisk(); + nbIntervals = nodeFreeSpace / 10 / intervalSize; start = fillValues(ht, STRING_VALUE, nbIntervals, start); - assertEquals(nodeFreeSpace - nbIntervals * (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING), node.getNodeFreeSpace()); + assertEquals(nodeFreeSpace - nbIntervals * intervalSize , node.getNodeFreeSpace()); } @@ -193,7 +202,7 @@ public class HistoryTreeTest { /* Fill a first node */ HTNode node = ht.getLatestLeaf(); int nodeFreeSpace = node.getNodeFreeSpace(); - int nbIntervals = nodeFreeSpace / (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING); + int nbIntervals = nodeFreeSpace / (STRING_INTERVAL.getSizeOnDisk()); long start = fillValues(ht, STRING_VALUE, nbIntervals, 1); /* Add intervals that should add a sibling to the node */ @@ -206,7 +215,7 @@ public class HistoryTreeTest { /* Fill the latest leaf node (2nd child) */ node = ht.getLatestLeaf(); nodeFreeSpace = node.getNodeFreeSpace(); - nbIntervals = nodeFreeSpace / (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING); + nbIntervals = nodeFreeSpace / (STRING_INTERVAL.getSizeOnDisk()); start = fillValues(ht, STRING_VALUE, nbIntervals, start); /* @@ -219,7 +228,7 @@ public class HistoryTreeTest { /* Fill the latest leaf node (3rd and last child) */ node = ht.getLatestLeaf(); nodeFreeSpace = node.getNodeFreeSpace(); - nbIntervals = nodeFreeSpace / (HTInterval.DATA_ENTRY_SIZE + TEST_STRING.length() + STRING_PADDING); + nbIntervals = nodeFreeSpace / (STRING_INTERVAL.getSizeOnDisk()); start = fillValues(ht, STRING_VALUE, nbIntervals, start); /* The new node created here should generate a new branch */ diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HTInterval.java b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HTInterval.java index dc5a31d996..99abcf5588 100644 --- a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HTInterval.java +++ b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HTInterval.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.tracecompass.statesystem.core.exceptions.StateValueTypeException; import org.eclipse.tracecompass.statesystem.core.exceptions.TimeRangeException; import org.eclipse.tracecompass.statesystem.core.interval.ITmfStateInterval; import org.eclipse.tracecompass.statesystem.core.statevalue.ITmfStateValue; @@ -35,18 +34,6 @@ public final class HTInterval implements ITmfStateInterval, Comparable - * 16 2 x Timevalue/long (interval start + end) - * + 4 int (key) - * + 1 byte (type) - * + 4 int (valueOffset) - * - */ - public static final int DATA_ENTRY_SIZE = 25; - /* 'Byte' equivalent for state values types */ private static final byte TYPE_NULL = -1; private static final byte TYPE_INTEGER = 0; @@ -54,21 +41,13 @@ public final class HTInterval implements ITmfStateInterval, Comparable + *
  • start (8 bytes)
    • + *
  • end (8 bytes)
    • + *
  • attribute (4 bytes)
    • + *
  • sv type (1 byte)
    • + *
  • sv ( 0 bytes for null, 4 for int , 8 for long and double, and the + * length of the string +2 for strings (it's variable))
    • + * + * * @param buffer * The ByteBuffer from which to read the information * @return The interval object @@ -132,10 +156,10 @@ public final class HTInterval implements ITmfStateInterval, Comparable + *
  • start (8 bytes)
    • + *
  • end (8 bytes)
    • + *
  • attribute (4 bytes)
    • + *
  • sv type (1 byte)
    • + *
  • sv ( 0 bytes for null, 4 for int , 8 for long and double, and the + * length of the string +2 for strings (it's variable))
    • + * + * * @param buffer * The already-allocated ByteBuffer corresponding to a SHT Node - * @param endPosOfStringEntry - * The initial (before calling this function for this interval) - * position of the Strings Entry for this node. This will change - * from one call to the other if we're writing String - * StateValues. - * @return The size of the Strings Entry that was written, if any. */ - public int writeInterval(ByteBuffer buffer, int endPosOfStringEntry) { + public void writeInterval(ByteBuffer buffer) { + final byte byteFromType = getByteFromType(sv.getType()); + buffer.putLong(start); buffer.putLong(end); buffer.putInt(attribute); - buffer.put(getByteFromType(sv.getType())); - - switch (getByteFromType(sv.getType())) { + buffer.put(byteFromType); + switch (byteFromType) { case TYPE_NULL: + break; case TYPE_INTEGER: - /* We write the 'valueOffset' field as a straight value. */ - try { - buffer.putInt(sv.unboxInt()); - } catch (StateValueTypeException e) { - /* - * This should not happen, since the value told us it was of - * type Null or Integer (corrupted value?) - */ - e.printStackTrace(); - } + buffer.putInt(sv.unboxInt()); break; case TYPE_STRING: - byte[] byteArrayToWrite; - try { - byteArrayToWrite = sv.unboxStr().getBytes(); - } catch (StateValueTypeException e1) { - /* Should not happen, we're in a switch/case for string type */ - throw new RuntimeException(); - } - - /* we use the valueOffset as an offset. */ - buffer.putInt(endPosOfStringEntry - stringsEntrySize); - buffer.mark(); - buffer.position(endPosOfStringEntry - stringsEntrySize); + String string = sv.unboxStr(); + byte[] strArray = string.getBytes(); /* - * write the Strings entry (1st byte = size, then the bytes, then the 0) + * Write the Strings entry (1st byte = size, then the bytes, then + * the 0). We have checked the string length at the constructor. */ - buffer.put((byte) stringsEntrySize); - buffer.put(byteArrayToWrite); + buffer.put((byte) strArray.length); + buffer.put(strArray); buffer.put((byte) 0); - assert (buffer.position() == endPosOfStringEntry); - buffer.reset(); break; case TYPE_LONG: - /* we use the valueOffset as an offset. */ - buffer.putInt(endPosOfStringEntry - stringsEntrySize); - buffer.mark(); - buffer.position(endPosOfStringEntry - stringsEntrySize); - - /* - * write the Long in the Strings section - */ - try { - buffer.putLong(sv.unboxLong()); - } catch (StateValueTypeException e) { - /* - * This should not happen, since the value told us it was of - * type Long (corrupted value?) - */ - e.printStackTrace(); - } - assert (buffer.position() == endPosOfStringEntry); - buffer.reset(); + buffer.putLong(sv.unboxLong()); break; case TYPE_DOUBLE: - /* we use the valueOffset as an offset. */ - buffer.putInt(endPosOfStringEntry - stringsEntrySize); - buffer.mark(); - buffer.position(endPosOfStringEntry - stringsEntrySize); - - /* Write the Double in the Strings section */ - try { - buffer.putDouble(sv.unboxDouble()); - } catch (StateValueTypeException e) { - /* - * This should not happen, since the value told us it was of - * type Double (corrupted value?) - */ - e.printStackTrace(); - } - if (buffer.position() != endPosOfStringEntry) { - throw new IllegalStateException(); - } - buffer.reset(); + buffer.putDouble(sv.unboxDouble()); break; default: break; } - return stringsEntrySize; } @Override @@ -369,44 +305,13 @@ public final class HTInterval implements ITmfStateInterval, Comparable(); @@ -193,7 +189,6 @@ public abstract class HTNode { int seqNb = buffer.getInt(); int parentSeqNb = buffer.getInt(); int intervalCount = buffer.getInt(); - int stringSectionOffset = buffer.getInt(); buffer.get(); // TODO Used to be "isDone", to be removed from the header /* Now the rest of the header depends on the node type */ @@ -222,12 +217,11 @@ public abstract class HTNode { for (i = 0; i < intervalCount; i++) { HTInterval interval = HTInterval.readFrom(buffer); newNode.fIntervals.add(interval); - newNode.fSizeOfIntervalSection += HTInterval.DATA_ENTRY_SIZE; + newNode.fSizeOfIntervalSection += interval.getSizeOnDisk(); } /* Assign the node's other information we have read previously */ newNode.fNodeEnd = end; - newNode.fStringSectionOffset = stringSectionOffset; newNode.fIsOnDisk = true; return newNode; @@ -250,7 +244,6 @@ public abstract class HTNode { fRwl.readLock().lock(); try { final int blockSize = fConfig.getBlockSize(); - int curStringsEntryEndPos = blockSize; ByteBuffer buffer = ByteBuffer.allocate(blockSize); buffer.order(ByteOrder.LITTLE_ENDIAN); @@ -263,40 +256,22 @@ public abstract class HTNode { buffer.putInt(fSequenceNumber); buffer.putInt(fParentSequenceNumber); buffer.putInt(fIntervals.size()); - buffer.putInt(fStringSectionOffset); buffer.put((byte) 1); // TODO Used to be "isDone", to be removed from header /* Now call the inner method to write the specific header part */ writeSpecificHeader(buffer); /* Back to us, we write the intervals */ - for (HTInterval interval : fIntervals) { - int size = interval.writeInterval(buffer, curStringsEntryEndPos); - curStringsEntryEndPos -= size; - } + fIntervals.forEach(i -> i.writeInterval(buffer)); /* - * Write padding between the end of the Data section and the start - * of the Strings section (needed to fill the node in case there is - * no Strings section) + * Fill the rest with zeros */ - while (buffer.position() < fStringSectionOffset) { + while (buffer.position() < blockSize) { buffer.put((byte) 0); } - /* - * If the offsets were right, the size of the Strings section should - * be == to the expected size - */ - if (curStringsEntryEndPos != fStringSectionOffset) { - throw new IllegalStateException("Wrong size of Strings section: Actual: " + curStringsEntryEndPos + ", Expected: " + fStringSectionOffset); //$NON-NLS-1$ //$NON-NLS-2$ - } - /* Finally, write everything in the Buffer to disk */ - - // if we don't do this, flip() will lose what's after. - buffer.position(blockSize); - buffer.flip(); int res = fc.write(buffer); if (res != blockSize) { @@ -391,7 +366,7 @@ public abstract class HTNode { fRwl.writeLock().lock(); try { /* Just in case, should be checked before even calling this function */ - assert (newInterval.getIntervalSize() <= getNodeFreeSpace()); + assert (newInterval.getSizeOnDisk() <= getNodeFreeSpace()); /* Find the insert position to keep the list sorted */ int index = fIntervals.size(); @@ -400,10 +375,8 @@ public abstract class HTNode { } fIntervals.add(index, newInterval); - fSizeOfIntervalSection += HTInterval.DATA_ENTRY_SIZE; + fSizeOfIntervalSection += newInterval.getSizeOnDisk(); - /* Update the in-node offset "pointer" */ - fStringSectionOffset -= (newInterval.getStringsEntrySize()); } finally { fRwl.writeLock().unlock(); } @@ -577,7 +550,7 @@ public abstract class HTNode { */ public int getNodeFreeSpace() { fRwl.readLock().lock(); - int ret = fStringSectionOffset - getDataSectionEndOffset(); + int ret = fConfig.getBlockSize() - getDataSectionEndOffset(); fRwl.readLock().unlock(); return ret; diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HistoryTree.java b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HistoryTree.java index 1aa0c7528c..e4154496ac 100644 --- a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HistoryTree.java +++ b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/internal/statesystem/core/backend/historytree/HistoryTree.java @@ -51,7 +51,7 @@ public class HistoryTree { private static final int HISTORY_FILE_MAGIC_NUMBER = 0x05FFA900; /** File format version. Increment when breaking compatibility. */ - private static final int FILE_VERSION = 5; + private static final int FILE_VERSION = 6; // ------------------------------------------------------------------------ // Tree-specific configuration @@ -462,7 +462,7 @@ public class HistoryTree { HTNode targetNode = fLatestBranch.get(indexOfNode); /* Verify if there is enough room in this node to store this interval */ - if (interval.getIntervalSize() > targetNode.getNodeFreeSpace()) { + if (interval.getSizeOnDisk() > targetNode.getNodeFreeSpace()) { /* Nope, not enough room. Insert in a new sibling instead. */ addSiblingNode(indexOfNode); tryInsertAtNode(interval, fLatestBranch.size() - 1);