From 360f899ef7af0e716676bb6f81c476046a1c4a53 Mon Sep 17 00:00:00 2001 From: Etienne Bergeron Date: Mon, 9 Dec 2013 20:59:32 -0500 Subject: [PATCH] tmf: Avoid HistoryTree to expose its internal HT_IO node. The HT_IO node is an abstraction of the file accesses for a tree node. It should not be exposed to others classes. The history tree must act as a proxy for requests to HT_IO. This will ease modifications and maintenance of HT_IO. HT_IO node should not keep a back reference to the HistoryTree. This is a bad design. HT_IO node needs to know about HTConfig to perform its tasks. The HT_IO class contains method to read from disk. The concept of a LastBranch is related to the HistoryTree. Thus, readFromMemory is the same than calling HistoryTree.readNode(...) and readFromDisk is the same than calling HT_IO.readNode(...) Each layer is reponsible of its own caching mechanism. Change-Id: Ia835a03efa321cdefd170de002833e044d144ee0 Signed-off-by: Etienne Bergeron Reviewed-on: https://git.eclipse.org/r/19557 Reviewed-by: Alexandre Montplaisir IP-Clean: Alexandre Montplaisir Tested-by: Alexandre Montplaisir --- .../backends/historytree/HT_IO.java | 86 ++++++------------- .../backends/historytree/HistoryTree.java | 67 ++++++++++++--- .../historytree/HistoryTreeBackend.java | 21 ++--- 3 files changed, 89 insertions(+), 85 deletions(-) diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HT_IO.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HT_IO.java index 445e0dfc15..6bb9fd8cbc 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HT_IO.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HT_IO.java @@ -20,21 +20,22 @@ import java.nio.channels.ClosedChannelException; import java.nio.channels.FileChannel; /** - * This class exists mainly for code isolation/clarification purposes. It - * contains all the methods and descriptors to handle reading/writing to the - * tree-file on disk and all the caching mechanisms. Every HistoryTree should - * contain 1 and only 1 HT_IO element. + * This class abstracts inputs/outputs of the HistoryTree nodes. + * + * It contains all the methods and descriptors to handle reading/writing nodes + * to the tree-file on disk and all the caching mechanisms. + * + * This abstraction is mainly for code isolation/clarification purposes. + * Every HistoryTree must contain 1 and only 1 HT_IO element. * * @author Alexandre Montplaisir * */ class HT_IO { - - /* reference to the tree to which this IO-object belongs */ - private final HistoryTree tree; + /* Configuration of the History Tree */ + private final HTConfig fConfig; /* Fields related to the file I/O */ - private final File historyTreeFile; private final FileInputStream fis; private final FileOutputStream fos; private final FileChannel fcIn; @@ -47,17 +48,20 @@ class HT_IO { /** * Standard constructor * - * @param tree + * @param config + * The configuration object for the StateHistoryTree * @param newFile - * Are we creating a new file from scratch? + * Flag indicating that the file must be created from scratch + * @throws IOException + * An exception can be thrown when file cannot be accessed */ - HT_IO(HistoryTree tree, boolean newFile) throws IOException { - this.tree = tree; - historyTreeFile = tree.getConfig().getStateFile(); - boolean success1 = true; + HT_IO(HTConfig config, boolean newFile) throws IOException { + fConfig = config; + File historyTreeFile = config.getStateFile(); if (newFile) { + boolean success1 = true; /* Create a new empty History Tree file */ if (historyTreeFile.exists()) { success1 = historyTreeFile.delete(); @@ -82,33 +86,6 @@ class HT_IO { this.fcOut = fos.getChannel(); } - /** - * Generic "read node" method, which checks if the node is in memory first, - * and if it's not it goes to disk to retrieve it. - * - * @param seqNumber - * Sequence number of the node we want - * @return The wanted node in object form - * @throws ClosedChannelException - * If the channel was closed before we could read - */ - HTNode readNode(int seqNumber) throws ClosedChannelException { - HTNode node = readNodeFromMemory(seqNumber); - if (node == null) { - return readNodeFromDisk(seqNumber); - } - return node; - } - - private HTNode readNodeFromMemory(int seqNumber) { - for (HTNode node : tree.getLatestBranch()) { - if (node.getSequenceNumber() == seqNumber) { - return node; - } - } - return null; - } - /** * This method here isn't private, if we know for sure the node cannot be in * memory it's a bit faster to use this directly (when opening a file from @@ -119,7 +96,7 @@ class HT_IO { * reading. Instead of using a big reader-writer lock, we'll * just catch this exception. */ - synchronized HTNode readNodeFromDisk(int seqNumber) throws ClosedChannelException { + synchronized HTNode readNode(int seqNumber) throws ClosedChannelException { /* Do a cache lookup */ int offset = seqNumber & (CACHE_SIZE - 1); HTNode readNode = fNodeCache[offset]; @@ -130,7 +107,7 @@ class HT_IO { /* Lookup on disk */ try { seekFCToNodePos(fcIn, seqNumber); - readNode = HTNode.readNode(tree.getConfig(), fcIn); + readNode = HTNode.readNode(fConfig, fcIn); /* Put the node in the cache. */ fNodeCache[offset] = readNode; @@ -164,28 +141,19 @@ class HT_IO { return this.fcOut; } - FileInputStream supplyATReader() { + FileInputStream supplyATReader(int nodeOffset) { try { /* * Position ourselves at the start of the Mapping section in the * file (which is right after the Blocks) */ - seekFCToNodePos(fcIn, tree.getNodeCount()); + seekFCToNodePos(fcIn, nodeOffset); } catch (IOException e) { e.printStackTrace(); } return fis; } - File supplyATWriterFile() { - return tree.getConfig().getStateFile(); - } - - long supplyATWriterFilePos() { - return HistoryTree.TREE_HEADER_SIZE - + ((long) tree.getNodeCount() * tree.getConfig().getBlockSize()); - } - synchronized void closeFile() { try { fis.close(); @@ -198,6 +166,7 @@ class HT_IO { synchronized void deleteFile() { closeFile(); + File historyTreeFile = fConfig.getStateFile(); if (!historyTreeFile.delete()) { /* We didn't succeed in deleting the file */ //TODO log it? @@ -208,17 +177,18 @@ class HT_IO { * Seek the given FileChannel to the position corresponding to the node that * has seqNumber * - * @param seqNumber + * @param fc the channel to seek + * @param seqNumber the node sequence number to seek the channel to * @throws IOException */ private void seekFCToNodePos(FileChannel fc, int seqNumber) throws IOException { - fc.position(HistoryTree.TREE_HEADER_SIZE + (long) seqNumber - * tree.getConfig().getBlockSize()); /* - * cast to (long) is needed to make sure the result is a long too and + * Cast to (long) is needed to make sure the result is a long too and * doesn't get truncated */ + fc.position(HistoryTree.TREE_HEADER_SIZE + + ((long) seqNumber) * fConfig.getBlockSize()); } } diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTree.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTree.java index fc8c382e97..9faa62b36b 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTree.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTree.java @@ -65,7 +65,7 @@ class HistoryTree { /** Latest timestamp found in the tree (at any given moment) */ private long treeEnd; - /** How many nodes exist in this tree, total */ + /** The total number of nodes that exists in this tree */ private int nodeCount; /** "Cache" to keep the active nodes in memory */ @@ -94,7 +94,7 @@ class HistoryTree { latestBranch = new ArrayList(); /* Prepare the IO object */ - treeIO = new HT_IO(this, true); + treeIO = new HT_IO(config, true); /* Add the first node to the tree */ CoreNode firstNode = initNewCoreNode(-1, conf.getTreeStart()); @@ -137,7 +137,7 @@ class HistoryTree { buffer.flip(); /* - * Check the magic number,to make sure we're opening the right type of + * Check the magic number to make sure we're opening the right type of * file */ res = buffer.getInt(); @@ -158,9 +158,11 @@ class HistoryTree { if (res != expProviderVersion && expProviderVersion != ITmfStateProvider.IGNORE_PROVIDER_VERSION) { /* - * The existing history was built using a event handler that doesn't - * match the current one in the framework. Information could be all - * wrong, so we'll force a rebuild of the history file instead. + * The existing history was built using an event handler that doesn't + * match the current one in the framework. + * + * Information could be all wrong. Instead of keeping an incorrect + * history file, a rebuild is done. */ fc.close(); fis.close(); @@ -182,7 +184,7 @@ class HistoryTree { * file, not extremely elegant. But how to pass the information here to * the SHT otherwise? */ - this.treeIO = new HT_IO(this, false); + this.treeIO = new HT_IO(config, false); rebuildLatestBranch(rootNodeSeqNb); this.treeEnd = latestBranch.get(0).getNodeEnd(); @@ -289,14 +291,51 @@ class HistoryTree { return nodeCount; } - HT_IO getTreeIO() { - return treeIO; - } - List getLatestBranch() { return Collections.unmodifiableList(latestBranch); } + // ------------------------------------------------------------------------ + // HT_IO interface + // ------------------------------------------------------------------------ + + File supplyATWriterFile() { + return config.getStateFile(); + } + + FileInputStream supplyATReader() { + return treeIO.supplyATReader(getNodeCount()); + } + + long supplyATWriterFilePos() { + return HistoryTree.TREE_HEADER_SIZE + + ((long) getNodeCount() * config.getBlockSize()); + } + + HTNode readNode(int seqNumber) throws ClosedChannelException { + /* Try to read the node from memory */ + for (HTNode node : getLatestBranch()) { + if (node.getSequenceNumber() == seqNumber) { + return node; + } + } + + /* Read the node from disk */ + return treeIO.readNode(seqNumber); + } + + void writeNode(HTNode node) { + treeIO.writeNode(node); + } + + void closeFile() { + treeIO.closeFile(); + } + + void deleteFile() { + treeIO.deleteFile(); + } + // ------------------------------------------------------------------------ // Operations // ------------------------------------------------------------------------ @@ -316,10 +355,10 @@ class HistoryTree { this.latestBranch = new ArrayList(); - nextChildNode = treeIO.readNodeFromDisk(rootNodeSeqNb); + nextChildNode = treeIO.readNode(rootNodeSeqNb); latestBranch.add((CoreNode) nextChildNode); while (latestBranch.get(latestBranch.size() - 1).getNbChildren() > 0) { - nextChildNode = treeIO.readNodeFromDisk(latestBranch.get(latestBranch.size() - 1).getLatestChild()); + nextChildNode = treeIO.readNode(latestBranch.get(latestBranch.size() - 1).getLatestChild()); latestBranch.add((CoreNode) nextChildNode); } } @@ -516,7 +555,7 @@ class HistoryTree { * node has to be on disk */ if (currentNode.isDone()) { - return treeIO.readNodeFromDisk(potentialNextSeqNb); + return treeIO.readNode(potentialNextSeqNb); } return treeIO.readNode(potentialNextSeqNb); } diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTreeBackend.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTreeBackend.java index ea1c611498..a1723c9e6f 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTreeBackend.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HistoryTreeBackend.java @@ -30,7 +30,7 @@ import org.eclipse.linuxtools.tmf.core.statevalue.TmfStateValue; * History Tree backend for storing a state history. This is the basic version * that runs in the same thread as the class creating it. * - * @author alexmont + * @author Alexandre Montplaisir * */ public class HistoryTreeBackend implements IStateHistoryBackend { @@ -38,9 +38,6 @@ public class HistoryTreeBackend implements IStateHistoryBackend { /** The history tree that sits underneath */ protected final HistoryTree sht; - /** Direct reference to the tree's IO object */ - private final HT_IO treeIO; - /** Indicates if the history tree construction is done */ protected boolean isFinishedBuilding = false; @@ -70,7 +67,6 @@ public class HistoryTreeBackend implements IStateHistoryBackend { final HTConfig conf = new HTConfig(newStateFile, blockSize, maxChildren, providerVersion, startTime); sht = new HistoryTree(conf); - treeIO = sht.getTreeIO(); } /** @@ -110,7 +106,6 @@ public class HistoryTreeBackend implements IStateHistoryBackend { public HistoryTreeBackend(File existingStateFile, int providerVersion) throws IOException { sht = new HistoryTree(existingStateFile, providerVersion); - treeIO = sht.getTreeIO(); isFinishedBuilding = true; } @@ -142,35 +137,35 @@ public class HistoryTreeBackend implements IStateHistoryBackend { @Override public FileInputStream supplyAttributeTreeReader() { - return treeIO.supplyATReader(); + return sht.supplyATReader(); } @Override public File supplyAttributeTreeWriterFile() { - return treeIO.supplyATWriterFile(); + return sht.supplyATWriterFile(); } @Override public long supplyAttributeTreeWriterFilePosition() { - return treeIO.supplyATWriterFilePos(); + return sht.supplyATWriterFilePos(); } @Override public void removeFiles() { - treeIO.deleteFile(); + sht.deleteFile(); } @Override public void dispose() { if (isFinishedBuilding) { - treeIO.closeFile(); + sht.closeFile(); } else { /* * The build is being interrupted, delete the file we partially * built since it won't be complete, so shouldn't be re-used in the * future (.deleteFile() will close the file first) */ - treeIO.deleteFile(); + sht.deleteFile(); } } @@ -281,7 +276,7 @@ public class HistoryTreeBackend implements IStateHistoryBackend { try { for (int seq = 0; seq < sht.getNodeCount(); seq++) { - node = treeIO.readNode(seq); + node = sht.readNode(seq); total += node.getNodeUsagePRC(); } } catch (ClosedChannelException e) { -- 2.34.1