Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (revision 81d2780d5da04237b88ac95aa6f5bd2359e8cc39) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (revision ) @@ -37,17 +37,35 @@ * A {@code MemoryNodeBuilder} instance tracks uncommitted changes without * relying on weak references or requiring hard references on the entire * accessed subtree. It does this by relying on {@code MutableNodeState} - * instances for tracking uncommitted changes. A child builders - * keeps a reference to its parent builder and knows its own name. Before - * each access the builder checks the mutable state of its parent for - * relevant changes and updates its own mutable state. + * instances for tracking uncommitted changes and on {@code Head} + * instances for tracking the connectedness of the builder. A builder keeps + * a reference to the parent builder and knows its own name, which is used + * to check for relevant changes in its parent builder and update its state + * accordingly. *

- * A {@code MutableNodeState} instance does not keep a reference to its - * parent state. It only keeps references to children that have been - * modified. Instances representing an unmodified child are created on - * the fly without keeping a reference. This effectively ensures that - * such an instance can be GC'ed once no node builder references it - * anymore. + * A builder is in one of three possible states, which is tracked within + * its {@code Head} instance: + *

+ *
unconnected
+ *
+ * A child builder with no content changes starts in this state. + * Before each access the unconnected builder checks its parent for + * relevant changes. + *
+ *
connected
+ *
+ * Once a builder is first modified, it switches to the connected state + * and records all modification in a shared {@code MutableNodeState} + * instance. Before each access the connected builder checks whether its + * parents base state has been reset and if so, resets its own base state + * accordingly. + *
+ *
root
+ *
+ * Same as the connected state but only the root of the builder hierarchy + * can have this state. + *
+ *
*/ public class MemoryNodeBuilder implements NodeBuilder { @@ -63,9 +81,9 @@ private final String name; /** - * Root builder, or {@code this} for the root itself. + * Root builder, or {@code this} for the root builder itself. */ - private final MemoryNodeBuilder root; + private final MemoryNodeBuilder rootBuilder; /** * Internal revision counter for the base state of this builder. The counter @@ -73,32 +91,22 @@ * Each builder instance has its own copy of this revision counter for * quickly checking whether its base state needs updating. * @see #reset(org.apache.jackrabbit.oak.spi.state.NodeState) + * @see #base() */ private long baseRevision; /** * The base state of this builder, possibly non-existent if this builder * represents a new node that didn't yet exist in the base content tree. - * @see #base() */ private NodeState base; /** - * Internal revision counter for the head state of this builder. The counter - * is incremented in the root builder whenever anything changes in the tree - * below. Each builder instance has its own copy of this revision counter for - * quickly checking whether its head state needs updating. + * Head of this builder */ - private long headRevision; + private Head head; /** - * The shared mutable state this builder. - * @see #write() - * @see #read() - */ - private MutableNodeState head; - - /** * Creates a new in-memory child builder. * @param parent parent builder * @param name name of this node @@ -106,7 +114,8 @@ private MemoryNodeBuilder(MemoryNodeBuilder parent, String name) { this.parent = parent; this.name = name; - this.root = parent.root; + this.rootBuilder = parent.rootBuilder; + this.head = new UnconnectedHead(); } /** @@ -117,83 +126,39 @@ public MemoryNodeBuilder(@Nonnull NodeState base) { this.parent = null; this.name = null; - this.root = this; + this.rootBuilder = this; - // ensure base is updated on next call to base() + // ensure child builder's base is updated on first access this.baseRevision = 1; this.base = checkNotNull(base); - // ensure head is updated on next call to read() or write() - this.headRevision = 1; - this.head = new MutableNodeState(this.base); + this.head = new RootHead(); } + /** + * @return {@code true} iff this is the root builder + */ private boolean isRoot() { - return this == root; + return this == rootBuilder; } /** * Update the base state of this builder by recursively retrieving it - * from the parent builder. + * from its parent builder. * @return base state of this builder */ @Nonnull private NodeState base() { - if (root.baseRevision != baseRevision) { + if (rootBuilder.baseRevision != baseRevision) { base = parent.base().getChildNode(name); - baseRevision = root.baseRevision; + baseRevision = rootBuilder.baseRevision; } return base; } /** - * Update the head state of this builder by recursively retrieving it - * from the parent builder. - * @return head state of this builder - */ - @Nonnull - private MutableNodeState read() { - if (headRevision != root.headRevision) { - assert !isRoot() : "root should have headRevision == root.headRevision"; - head = parent.read().getChildNode(name, false); - headRevision = root.headRevision; - } - return head; - } - - /** - * Update the head state of this builder by recursively retrieving it - * from the parent builder and increment the head revision of the root - * builder ensuring subsequent calls to {@link #read()} result in updating - * of the respective head states. - * @return head state of this builder - */ - @Nonnull - private MutableNodeState write() { - // TODO avoid traversing the parent hierarchy twice: once for exist and once for write - if (!exists()) { - throw new IllegalStateException("This builder does not exist: " + name); - } - return write(root.headRevision + 1); - } - - /** - * Recursive helper method to {@link #write()}. Don't call directly. - */ - @Nonnull - private MutableNodeState write(long newRevision) { - if (headRevision != newRevision && !isRoot()) { - head = parent.write(newRevision).getChildNode(name, true); - headRevision = newRevision; - } - root.headRevision = newRevision; - return head; - } - - /** * Factory method for creating new child state builders. Subclasses may * override this method to control the behavior of child state builders. - * * @return new builder */ protected MemoryNodeBuilder createChildBuilder(String name) { @@ -215,7 +180,7 @@ @Override public NodeState getNodeState() { - return read().snapshot(); + return head.getNodeState(); } @Override @@ -225,7 +190,7 @@ @Override public boolean exists() { - return read().exists(); + return head.exists(); } @Override @@ -235,31 +200,29 @@ @Override public boolean isModified() { - return read().isModified(base()); + return head.isModified(base()); } @Override public void reset(NodeState newBase) { - checkState(isRoot(), "Cannot reset a non-root builder"); base = checkNotNull(newBase); - root.baseRevision++; - root.headRevision++; - head = new MutableNodeState(base); + baseRevision++; + head.reset(); } @Override public long getChildNodeCount() { - return read().getChildNodeCount(); + return head.getChildNodeCount(); } @Override public Iterable getChildNodeNames() { - return read().getChildNodeNames(); + return head.getChildNodeNames(); } @Override public boolean hasChildNode(String name) { - return read().hasChildNode(checkNotNull(name)); + return head.hasChildNode(checkNotNull(name)); } @Override @@ -283,7 +246,8 @@ @Override public NodeBuilder setChildNode(String name, NodeState state) { - write().setChildNode(checkNotNull(name), checkNotNull(state)); + checkState(exists(), "This builder does not exist: " + name); + head.setChildNode(checkNotNull(name), checkNotNull(state)); MemoryNodeBuilder builder = createChildBuilder(name); updated(); return builder; @@ -291,34 +255,32 @@ @Override public boolean remove() { - if (!exists()) { - return false; - } else { - write(); - parent.head.removeChildNode(name); - updated(); + if (exists()) { + head.remove(); return true; + } else { + return false; } } @Override public long getPropertyCount() { - return read().getPropertyCount(); + return head.getPropertyCount(); } @Override public Iterable getProperties() { - return read().getProperties(); + return head.getProperties(); } @Override public boolean hasProperty(String name) { - return read().hasProperty(checkNotNull(name)); + return head.hasProperty(checkNotNull(name)); } @Override public PropertyState getProperty(String name) { - return read().getProperty(checkNotNull(name)); + return head.getProperty(checkNotNull(name)); } @Override @@ -351,7 +313,8 @@ @Override public NodeBuilder setProperty(PropertyState property) { - write().setProperty(checkNotNull(property)); + checkState(exists(), "This builder does not exist: " + name); + head.setProperty(checkNotNull(property)); updated(); return this; } @@ -370,10 +333,159 @@ @Override public NodeBuilder removeProperty(String name) { - if (write().removeProperty(checkNotNull(name))) { + checkState(exists(), "This builder does not exist: " + name); + if (head.removeProperty(checkNotNull(name))) { updated(); } return this; + } + + //------------------------------------------------------------< Head >--- + + /** + * The subclasses of this abstract base class represent the different + * states builders can have: unconnected, connected, + * and root. {@code MemoryNodeBuilder} implements most of its + * functionality by forwarding to the methods of {@code Head} instances + * where the actual type of {@code Head} determines the behaviour associated + * with the current state. + */ + private abstract class Head { + protected long revision; + protected NodeState state; + + protected abstract NodeState read(); + protected abstract MutableNodeState write(); + + public NodeState getNodeState() { + NodeState state = read(); + return state instanceof MutableNodeState + ? ((MutableNodeState) state).snapshot() + : state; + } + + public boolean exists() { + return read().exists(); + } + + public boolean isModified(NodeState base) { + NodeState state = read(); + return state instanceof MutableNodeState && ((MutableNodeState) state).isModified(base); + } + + public void reset() { + throw new IllegalStateException("Cannot reset a non-root builder"); + } + + public long getChildNodeCount() { + return read().getChildNodeCount(); + } + + public Iterable getChildNodeNames() { + return read().getChildNodeNames(); + } + + public boolean hasChildNode(String name) { + return read().hasChildNode(name); + } + + public void setChildNode(String name, NodeState nodeState) { + write().setChildNode(name, nodeState); + } + + public void remove() { + head.write(); + parent.head.write().removeChildNode(name); + } + + public long getPropertyCount() { + return read().getPropertyCount(); + } + + public Iterable getProperties() { + return read().getProperties(); + } + + public boolean hasProperty(String name) { + return read().hasProperty(name); + } + + public PropertyState getProperty(String name) { + return read().getProperty(name); + } + + public void setProperty(PropertyState propertyState) { + write().setProperty(propertyState); + } + + public boolean removeProperty(String name) { + return write().removeProperty(name); + } + } + + private class ConnectedHead extends Head { + @Override + protected MutableNodeState read() { + if (revision != rootBuilder.baseRevision) { + // the root builder's base state has been reset: re-get + // state from parent. + MutableNodeState parentState = (MutableNodeState) parent.head.read(); + state = parentState.getMutableChildNode(name); + revision = rootBuilder.baseRevision; + } + return (MutableNodeState) state; + } + + @Override + protected MutableNodeState write() { + // incrementing the root revision triggers unconnected + // child state to re-get their state on next access + rootBuilder.head.revision++; + return read(); + } + } + + private class UnconnectedHead extends Head { + @Override + protected NodeState read() { + if (revision != rootBuilder.head.revision) { + // root revision changed: recursively re-get state from parent + NodeState parentState = parent.head.read(); + state = parentState.getChildNode(name); + revision = rootBuilder.head.revision; + } + return state; + } + + @Override + protected MutableNodeState write() { + // switch to connected state recursively up to the parent + parent.head.write(); + return (head = new ConnectedHead()).write(); + } + } + + private class RootHead extends Head { + public RootHead() { + // ensure updating of child builders on first access + reset(); + } + + @Override + protected MutableNodeState read() { + return (MutableNodeState) state; + } + + @Override + protected MutableNodeState write() { + return (MutableNodeState) state; + } + + @Override + public final void reset() { + state = new MutableNodeState(base); + revision++; + } } } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java (revision 81d2780d5da04237b88ac95aa6f5bd2359e8cc39) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MutableNodeState.java (revision ) @@ -83,23 +83,6 @@ } /** - * Get and optionally connect a potentially non existing child - * node of a given {@code name}. Connected child nodes are kept - * in the list of modified child nodes of this node. - */ - MutableNodeState getChildNode(String name, boolean connect) { - assert base != null; - MutableNodeState child = nodes.get(name); - if (child == null) { - child = new MutableNodeState(base.getChildNode(name)); - if (connect) { - nodes.put(name, child); - } - } - return child; - } - - /** * Equivalent to *
      *   MutableNodeState child = getChildNode(name, true);
@@ -269,9 +252,31 @@
         }
     }
 
+    /**
+     * Returns a mutable child node with the given name. If the named
+     * child node has already been modified, i.e. there's an entry for
+     * it in the {@link #nodes} map, then that child instance is returned
+     * directly. Otherwise a new mutable child instance is created based
+     * on the (possibly non-existent) respective child node of the base
+     * state, added to the {@link #nodes} map and returned.
+     */
+    MutableNodeState getMutableChildNode(String name) {
+        assert base != null;
+        MutableNodeState child = nodes.get(name);
+        if (child == null) {
+            child = new MutableNodeState(base.getChildNode(name));
+            nodes.put(name, child);
+        }
+        return child;
+    }
+
     @Override
-    public MutableNodeState getChildNode(String name) {
-        throw new UnsupportedOperationException();
+    public NodeState getChildNode(String name) {
+        NodeState child = nodes.get(name);
+        if (child == null) {
+            child = base.getChildNode(name);
+        }
+        return child;
     }
 
     @Override @Nonnull