You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Stefan Guggisberg <st...@gmail.com> on 2012/04/04 18:24:46 UTC
Re: svn commit: r1298366 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk:
./ model/ store/
i just realized (sorry, a bit late, i know) that this commit
breaks some of the optimizations for large child node lists.
comparison of nodes had previously been delegated
to the Node/ChildNodeEntries implementation by calling
Node.diff(Node other). this allowed for specialized handling of
large child node collections (see e.g. ChildNodeEnttriesTree.getAdded()).
now, with diffing implemented on top of the abstract tree model,
those optimizations are unfortunately lost and diffing large
collections of child node entries will be highly inefficient
and will significantly affect the performance of e.g. MK.getJournal()...
cheers
stefan
On Thu, Mar 8, 2012 at 12:35 PM, <ju...@apache.org> wrote:
> Author: jukka
> Date: Thu Mar 8 11:35:35 2012
> New Revision: 1298366
>
> URL: http://svn.apache.org/viewvc?rev=1298366&view=rev
> Log:
> OAK-3: Internal tree model
>
> Commit proposed patch to implement diffing and related operations on top of the abstract tree model.
>
> Added:
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java (with props)
> Modified:
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/MicroKernelImpl.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/AbstractNode.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/Node.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDiffHandler.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/MicroKernelImpl.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/MicroKernelImpl.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/MicroKernelImpl.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/MicroKernelImpl.java Thu Mar 8 11:35:35 2012
> @@ -20,14 +20,13 @@ import org.apache.jackrabbit.mk.api.Micr
> import org.apache.jackrabbit.mk.api.MicroKernelException;
> import org.apache.jackrabbit.mk.json.JsopBuilder;
> import org.apache.jackrabbit.mk.json.JsopTokenizer;
> -import org.apache.jackrabbit.mk.model.ChildNodeEntry;
> import org.apache.jackrabbit.mk.model.Commit;
> import org.apache.jackrabbit.mk.model.CommitBuilder;
> import org.apache.jackrabbit.mk.model.Id;
> import org.apache.jackrabbit.mk.model.StoredCommit;
> -import org.apache.jackrabbit.mk.model.StoredNode;
> import org.apache.jackrabbit.mk.model.TraversingNodeDiffHandler;
> import org.apache.jackrabbit.mk.store.NotFoundException;
> +import org.apache.jackrabbit.mk.store.RevisionProvider;
> import org.apache.jackrabbit.mk.util.CommitGate;
> import org.apache.jackrabbit.mk.util.PathUtils;
> import org.apache.jackrabbit.mk.util.SimpleLRUCache;
> @@ -207,19 +206,20 @@ public class MicroKernelImpl implements
>
> try {
> final JsopBuilder buff = new JsopBuilder();
> + final RevisionProvider rp = rep.getRevisionStore();
> // maps (key: id of target node, value: path/to/target)
> // for tracking added/removed nodes; this allows us
> // to detect 'move' operations
> final HashMap<Id, String> addedNodes = new HashMap<Id, String>();
> final HashMap<Id, String> removedNodes = new HashMap<Id, String>();
> - StoredNode node1, node2;
> + NodeState node1, node2;
> try {
> - node1 = rep.getNode(fromRevisionId, path);
> + node1 = rep.getNodeState(fromRevisionId, path);
> } catch (NotFoundException e) {
> node1 = null;
> }
> try {
> - node2 = rep.getNode(toRevisionId, path);
> + node2 = rep.getNodeState(toRevisionId, path);
> } catch (NotFoundException e) {
> node2 = null;
> }
> @@ -227,8 +227,7 @@ public class MicroKernelImpl implements
> if (node1 == null) {
> if (node2 != null) {
> buff.tag('+').key(path).object();
> - toJson(buff, rep.getRevisionStore().getNodeState(node2),
> - Integer.MAX_VALUE, 0, -1, false);
> + toJson(buff, node2, Integer.MAX_VALUE, 0, -1, false);
> return buff.endObject().newline().toString();
> } else {
> throw new MicroKernelException("path doesn't exist in the specified revisions: " + path);
> @@ -239,52 +238,48 @@ public class MicroKernelImpl implements
> return buff.newline().toString();
> }
>
> - TraversingNodeDiffHandler diffHandler = new TraversingNodeDiffHandler(rep.getRevisionStore()) {
> + TraversingNodeDiffHandler diffHandler = new TraversingNodeDiffHandler() {
> @Override
> - public void propAdded(String propName, String value) {
> + public void propertyAdded(PropertyState after) {
> buff.tag('+').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> - encodedValue(value).
> + key(PathUtils.concat(getCurrentPath(), after.getName())).
> + encodedValue(after.getEncodedValue()).
> newline();
> }
>
> @Override
> - public void propChanged(String propName, String oldValue, String newValue) {
> + public void propertyChanged(PropertyState before, PropertyState after) {
> buff.tag('^').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> - encodedValue(newValue).
> + key(PathUtils.concat(getCurrentPath(), after.getName())).
> + encodedValue(after.getEncodedValue()).
> newline();
> }
>
> @Override
> - public void propDeleted(String propName, String value) {
> + public void propertyDeleted(PropertyState before) {
> // since property and node deletions can't be distinguished
> // using the "- <path>" notation we're representing
> // property deletions as "^ <path>:null"
> buff.tag('^').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> + key(PathUtils.concat(getCurrentPath(), before.getName())).
> value(null).
> newline();
> }
>
> @Override
> - public void childNodeAdded(ChildNodeEntry added) {
> - addedNodes.put(added.getId(), PathUtils.concat(getCurrentPath(), added.getName()));
> + public void childNodeAdded(String name, NodeState after) {
> + addedNodes.put(rp.getId(after), PathUtils.concat(getCurrentPath(), name));
> buff.tag('+').
> - key(PathUtils.concat(getCurrentPath(), added.getName())).object();
> - try {
> - toJson(buff, store.getNodeState(store.getNode(added.getId())), Integer.MAX_VALUE, 0, -1, false);
> - } catch (Exception e) {
> - buff.value("ERROR: failed to retrieve node " + added.getId());
> - }
> + key(PathUtils.concat(getCurrentPath(), name)).object();
> + toJson(buff, after, Integer.MAX_VALUE, 0, -1, false);
> buff.endObject().newline();
> }
>
> @Override
> - public void childNodeDeleted(ChildNodeEntry deleted) {
> - removedNodes.put(deleted.getId(), PathUtils.concat(getCurrentPath(), deleted.getName()));
> + public void childNodeDeleted(String name, NodeState before) {
> + removedNodes.put(rp.getId(before), PathUtils.concat(getCurrentPath(), name));
> buff.tag('-');
> - buff.value(PathUtils.concat(getCurrentPath(), deleted.getName()));
> + buff.value(PathUtils.concat(getCurrentPath(), name));
> buff.newline();
> }
> };
> @@ -303,58 +298,54 @@ public class MicroKernelImpl implements
>
> // TODO refactor code, avoid duplication
>
> - diffHandler = new TraversingNodeDiffHandler(rep.getRevisionStore()) {
> + diffHandler = new TraversingNodeDiffHandler() {
> @Override
> - public void propAdded(String propName, String value) {
> + public void propertyAdded(PropertyState after) {
> buff.tag('+').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> - encodedValue(value).
> + key(PathUtils.concat(getCurrentPath(), after.getName())).
> + encodedValue(after.getEncodedValue()).
> newline();
> }
>
> @Override
> - public void propChanged(String propName, String oldValue, String newValue) {
> + public void propertyChanged(PropertyState before, PropertyState after) {
> buff.tag('^').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> - encodedValue(newValue).
> + key(PathUtils.concat(getCurrentPath(), after.getName())).
> + encodedValue(after.getEncodedValue()).
> newline();
> }
>
> @Override
> - public void propDeleted(String propName, String value) {
> + public void propertyDeleted(PropertyState before) {
> // since property and node deletions can't be distinguished
> // using the "- <path>" notation we're representing
> // property deletions as "^ <path>:null"
> buff.tag('^').
> - key(PathUtils.concat(getCurrentPath(), propName)).
> + key(PathUtils.concat(getCurrentPath(), before.getName())).
> value(null).
> newline();
> }
>
> @Override
> - public void childNodeAdded(ChildNodeEntry added) {
> - if (addedNodes.containsKey(added.getId())) {
> + public void childNodeAdded(String name, NodeState after) {
> + if (addedNodes.containsKey(rp.getId(after))) {
> // moved node, will be processed separately
> return;
> }
> buff.tag('+').
> - key(PathUtils.concat(getCurrentPath(), added.getName())).object();
> - try {
> - toJson(buff, store.getNodeState(store.getNode(added.getId())), Integer.MAX_VALUE, 0, -1, false);
> - } catch (Exception e) {
> - buff.value("ERROR: failed to retrieve node " + added.getId());
> - }
> + key(PathUtils.concat(getCurrentPath(), name)).object();
> + toJson(buff, after, Integer.MAX_VALUE, 0, -1, false);
> buff.endObject().newline();
> }
>
> @Override
> - public void childNodeDeleted(ChildNodeEntry deleted) {
> - if (addedNodes.containsKey(deleted.getId())) {
> + public void childNodeDeleted(String name, NodeState before) {
> + if (addedNodes.containsKey(rp.getId(before))) {
> // moved node, will be processed separately
> return;
> }
> buff.tag('-');
> - buff.value(PathUtils.concat(getCurrentPath(), deleted.getName()));
> + buff.value(PathUtils.concat(getCurrentPath(), name));
> buff.newline();
> }
>
> @@ -596,7 +587,7 @@ public class MicroKernelImpl implements
>
> //-------------------------------------------------------< implementation >
>
> - void toJson(JsopBuilder builder, NodeState node, int depth, int offset, int count, boolean inclVirtualProps) throws Exception {
> + void toJson(JsopBuilder builder, NodeState node, int depth, int offset, int count, boolean inclVirtualProps) {
> for (PropertyState property : node.getProperties()) {
> builder.key(property.getName()).encodedValue(property.getEncodedValue());
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/AbstractNode.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/AbstractNode.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/AbstractNode.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/AbstractNode.java Thu Mar 8 11:35:35 2012
> @@ -131,81 +131,6 @@ public abstract class AbstractNode imple
> return node;
> }
>
> - public void diff(Node other, NodeDiffHandler handler) {
> - // compare properties
> - Map<String, String> oldProps = getProperties();
> - Map<String, String> newProps = other.getProperties();
> - if (!oldProps.equals(newProps)) {
> - Set<String> set = new HashSet<String>();
> - set.addAll(oldProps.keySet());
> - set.removeAll(newProps.keySet());
> - for (String name : set) {
> - handler.propDeleted(name, oldProps.get(name));
> - }
> - set.clear();
> - set.addAll(newProps.keySet());
> - set.removeAll(oldProps.keySet());
> - for (String name : set) {
> - handler.propAdded(name, newProps.get(name));
> - }
> - for (Map.Entry<String, String> entry : oldProps.entrySet()) {
> - String val = newProps.get(entry.getKey());
> - if (val != null && !entry.getValue().equals(val)) {
> - handler.propChanged(entry.getKey(), entry.getValue(), val);
> - }
> - }
> - }
> -
> - // compare child node entries
> -
> - if (other instanceof AbstractNode) {
> - ChildNodeEntries otherEntries = ((AbstractNode) other).childEntries;
> - for (Iterator<ChildNodeEntry> it = childEntries.getAdded(otherEntries); it.hasNext(); ) {
> - handler.childNodeAdded(it.next());
> - }
> - for (Iterator<ChildNodeEntry> it = childEntries.getRemoved(otherEntries); it.hasNext(); ) {
> - handler.childNodeDeleted(it.next());
> - }
> - for (Iterator<ChildNodeEntry> it = childEntries.getModified(otherEntries); it.hasNext(); ) {
> - ChildNodeEntry old = it.next();
> - ChildNodeEntry modified = otherEntries.get(old.getName());
> - handler.childNodeChanged(old, modified.getId());
> - }
> - return;
> - }
> -
> - Map<String, ChildNodeEntry> oldEntries = new HashMap<String, ChildNodeEntry>(getChildNodeCount());
> - for (Iterator<ChildNodeEntry> it = getChildNodeEntries(0, -1); it.hasNext(); ) {
> - ChildNodeEntry cne = it.next();
> - oldEntries.put(cne.getName(), cne);
> - }
> - Map<String, ChildNodeEntry> newEntries = new HashMap<String, ChildNodeEntry>(other.getChildNodeCount());
> - for (Iterator<ChildNodeEntry> it = other.getChildNodeEntries(0, -1); it.hasNext(); ) {
> - ChildNodeEntry cne = it.next();
> - newEntries.put(cne.getName(), cne);
> - }
> - if (!oldEntries.equals(newEntries)) {
> - Set<String> set = new HashSet<String>();
> - set.addAll(oldEntries.keySet());
> - set.removeAll(newEntries.keySet());
> - for (String name : set) {
> - handler.childNodeDeleted(oldEntries.get(name));
> - }
> - set.clear();
> - set.addAll(newEntries.keySet());
> - set.removeAll(oldEntries.keySet());
> - for (String name : set) {
> - handler.childNodeAdded(newEntries.get(name));
> - }
> - for (ChildNodeEntry cneOld : oldEntries.values()) {
> - ChildNodeEntry cneNew = newEntries.get(cneOld.getName());
> - if (cneNew != null && !cneNew.getId().equals(cneOld.getId())) {
> - handler.childNodeChanged(cneOld, cneNew.getId());
> - }
> - }
> - }
> - }
> -
> public void serialize(Binding binding) throws Exception {
> final Iterator<Map.Entry<String, String>> iter = properties.entrySet().iterator();
> binding.writeMap(":props", properties.size(),
> @@ -227,4 +152,4 @@ public abstract class AbstractNode imple
> binding.write(":inlined", childEntries.inlined() ? 1 : 0);
> childEntries.serialize(binding);
> }
> -}
> \ No newline at end of file
> +}
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/CommitBuilder.java Thu Mar 8 11:35:35 2012
> @@ -336,8 +336,10 @@ public class CommitBuilder {
> }
>
> void mergeNode(StoredNode baseNode, StoredNode ourNode, StoredNode theirNode, String path) throws Exception {
> - NodeDelta theirChanges = new NodeDelta(baseNode, theirNode);
> - NodeDelta ourChanges = new NodeDelta(baseNode, ourNode);
> + NodeDelta theirChanges = new NodeDelta(
> + store, store.getNodeState(baseNode), store.getNodeState(theirNode));
> + NodeDelta ourChanges = new NodeDelta(
> + store, store.getNodeState(baseNode), store.getNodeState(ourNode));
>
> // merge non-conflicting changes
> MutableNode mergedNode = new MutableNode(theirNode, store);
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/Node.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/Node.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/Node.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/Node.java Thu Mar 8 11:35:35 2012
> @@ -39,7 +39,5 @@ public interface Node {
> Iterator<ChildNode> getChildNodes(int offset, int count) throws Exception;
> Node getNode(String relPath) throws NotFoundException, Exception;
>
> - void diff(Node other, NodeDiffHandler handler);
> -
> void serialize(Binding binding) throws Exception;
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDelta.java Thu Mar 8 11:35:35 2012
> @@ -21,10 +21,14 @@ import java.util.HashMap;
> import java.util.List;
> import java.util.Map;
>
> +import org.apache.jackrabbit.mk.store.RevisionProvider;
> +import org.apache.jackrabbit.oak.model.NodeState;
> +import org.apache.jackrabbit.oak.model.PropertyState;
> +
> /**
> *
> */
> -public class NodeDelta implements NodeDiffHandler {
> +public class NodeDelta {
>
> public static enum ConflictType {
> /**
> @@ -47,8 +51,9 @@ public class NodeDelta implements NodeDi
> REMOVED_DIRTY_NODE_CONFLICT
> }
>
> - final StoredNode node1;
> - final StoredNode node2;
> + private final RevisionProvider provider;
> +
> + private final NodeState node1;
>
> Map<String, String> addedProperties = new HashMap<String, String>();
> Map<String, String> removedProperties = new HashMap<String, String>();
> @@ -58,11 +63,11 @@ public class NodeDelta implements NodeDi
> Map<String, Id> removedChildNodes = new HashMap<String, Id>();
> Map<String, Id> changedChildNodes = new HashMap<String, Id>();
>
> - public NodeDelta(StoredNode node1, StoredNode node2) throws Exception {
> + public NodeDelta(
> + RevisionProvider provider, NodeState node1, NodeState node2) {
> + this.provider = provider;
> this.node1 = node1;
> - this.node2 = node2;
> -
> - node1.diff(node2, this);
> + new DiffHandler().compare(node1, node2);
> }
>
> public Map<String, String> getAddedProperties() {
> @@ -95,7 +100,7 @@ public class NodeDelta implements NodeDi
>
> public List<Conflict> listConflicts(NodeDelta other) {
> // assume that both delta's were built using the *same* base node revision
> - if (!node1.getId().equals(other.node1.getId())) {
> + if (!node1.equals(other.node1)) {
> throw new IllegalArgumentException("other and this NodeDelta object are expected to share common node1 instance");
> }
>
> @@ -168,33 +173,42 @@ public class NodeDelta implements NodeDi
> return conflicts;
> }
>
> - //------------------------------------------------------< NodeDiffHandler >
> + //--------------------------------------------------------< inner classes >
>
> - public void propAdded(String propName, String value) {
> - addedProperties.put(propName, value);
> - }
> + private class DiffHandler extends NodeStateDiff {
>
> - public void propChanged(String propName, String oldValue, String newValue) {
> - changedProperties.put(propName, newValue);
> - }
> + @Override
> + public void propertyAdded(PropertyState after) {
> + addedProperties.put(after.getName(), after.getEncodedValue());
> + }
>
> - public void propDeleted(String propName, String value) {
> - removedProperties.put(propName, value);
> - }
> + @Override
> + public void propertyChanged(PropertyState before, PropertyState after) {
> + changedProperties.put(after.getName(), after.getEncodedValue());
> + }
>
> - public void childNodeAdded(ChildNodeEntry added) {
> - addedChildNodes.put(added.getName(), added.getId());
> - }
> + @Override
> + public void propertyDeleted(PropertyState before) {
> + removedProperties.put(before.getName(), before.getEncodedValue());
> + }
>
> - public void childNodeDeleted(ChildNodeEntry deleted) {
> - removedChildNodes.put(deleted.getName(), deleted.getId());
> - }
> + @Override
> + public void childNodeAdded(String name, NodeState after) {
> + addedChildNodes.put(name, provider.getId(after));
> + }
>
> - public void childNodeChanged(ChildNodeEntry changed, Id newId) {
> - changedChildNodes.put(changed.getName(), newId);
> - }
> + @Override
> + public void childNodeChanged(
> + String name, NodeState before, NodeState after) {
> + changedChildNodes.put(name, provider.getId(after));
> + }
>
> - //--------------------------------------------------------< inner classes >
> + @Override
> + public void childNodeDeleted(String name, NodeState before) {
> + removedChildNodes.put(name, provider.getId(before));
> + }
> +
> + }
>
> public static class Conflict {
>
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDiffHandler.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDiffHandler.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDiffHandler.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeDiffHandler.java Thu Mar 8 11:35:35 2012
> @@ -1,35 +0,0 @@
> -/*
> - * Licensed to the Apache Software Foundation (ASF) under one or more
> - * contributor license agreements. See the NOTICE file distributed with
> - * this work for additional information regarding copyright ownership.
> - * The ASF licenses this file to You under the Apache License, Version 2.0
> - * (the "License"); you may not use this file except in compliance with
> - * the License. You may obtain a copy of the License at
> - *
> - * http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -package org.apache.jackrabbit.mk.model;
> -
> -/**
> - *
> - */
> -public interface NodeDiffHandler {
> -
> - void propAdded(String propName, String value);
> -
> - void propChanged(String propName, String oldValue, String newValue);
> -
> - void propDeleted(String propName, String value);
> -
> - void childNodeAdded(ChildNodeEntry added);
> -
> - void childNodeDeleted(ChildNodeEntry deleted);
> -
> - void childNodeChanged(ChildNodeEntry changed, Id newId);
> -}
>
> Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java?rev=1298366&view=auto
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java (added)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java Thu Mar 8 11:35:35 2012
> @@ -0,0 +1,167 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.jackrabbit.mk.model;
> +
> +import java.util.HashSet;
> +import java.util.Set;
> +
> +import org.apache.jackrabbit.oak.model.ChildNodeEntry;
> +import org.apache.jackrabbit.oak.model.NodeState;
> +import org.apache.jackrabbit.oak.model.PropertyState;
> +
> +/**
> + * Utility base class for comparing two {@link NodeState} instances. The
> + * {@link #compare(NodeState, NodeState)} method will go through all
> + * properties and child nodes of the two states, calling the relevant
> + * added, changed or deleted methods where appropriate. Differences in
> + * the ordering of properties or child nodes do not affect the comparison.
> + */
> +public class NodeStateDiff {
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all added
> + * properties. The default implementation does nothing.
> + *
> + * @param after property state after the change
> + */
> + public void propertyAdded(PropertyState after) {
> + }
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all changed
> + * properties. The names of the given two property states are guaranteed
> + * to be the same. The default implementation does nothing.
> + *
> + * @param before property state before the change
> + * @param after property state after the change
> + */
> + public void propertyChanged(PropertyState before, PropertyState after) {
> + }
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all deleted
> + * properties. The default implementation does nothing.
> + *
> + * @param before property state before the change
> + */
> + public void propertyDeleted(PropertyState before) {
> + }
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all added
> + * child nodes. The default implementation does nothing.
> + *
> + * @param name name of the added child node
> + * @param after child node state after the change
> + */
> + public void childNodeAdded(String name, NodeState after) {
> + }
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all changed
> + * child nodes. The default implementation does nothing.
> + *
> + * @param name name of the changed child node
> + * @param before child node state before the change
> + * @param after child node state after the change
> + */
> + public void childNodeChanged(String name, NodeState before, NodeState after) {
> + }
> +
> + /**
> + * Called by {@link #compare(NodeState, NodeState)} for all deleted
> + * child nodes. The default implementation does nothing.
> + *
> + * @param name name of the deleted child node
> + * @param before child node state before the change
> + */
> + public void childNodeDeleted(String name, NodeState before) {
> + }
> +
> + /**
> + * Compares the given two node states. Any found differences are
> + * reported by calling the relevant added, changed or deleted methods.
> + *
> + * @param before node state before changes
> + * @param after node state after changes
> + */
> + public void compare(NodeState before, NodeState after) {
> + compareProperties(before, after);
> + compareChildNodes(before, after);
> + }
> +
> + /**
> + * Compares the properties of the given two node states.
> + *
> + * @param before node state before changes
> + * @param after node state after changes
> + */
> + private void compareProperties(NodeState before, NodeState after) {
> + Set<String> beforeProperties = new HashSet<String>();
> +
> + for (PropertyState beforeProperty : before.getProperties()) {
> + String name = beforeProperty.getName();
> + PropertyState afterProperty = after.getProperty(name);
> + if (afterProperty == null) {
> + propertyDeleted(beforeProperty);
> + } else {
> + beforeProperties.add(name);
> + if (!beforeProperty.equals(afterProperty)) {
> + propertyChanged(beforeProperty, afterProperty);
> + }
> + }
> + }
> +
> + for (PropertyState afterProperty : after.getProperties()) {
> + if (!beforeProperties.contains(afterProperty.getName())) {
> + propertyAdded(afterProperty);
> + }
> + }
> + }
> +
> + /**
> + * Compares the child nodes of the given two node states.
> + *
> + * @param before node state before changes
> + * @param after node state after changes
> + */
> + private void compareChildNodes(NodeState before, NodeState after) {
> + Set<String> beforeChildNodes = new HashSet<String>();
> +
> + for (ChildNodeEntry beforeCNE : before.getChildNodeEntries(0, -1)) {
> + String name = beforeCNE.getName();
> + NodeState beforeChild = beforeCNE.getNode();
> + NodeState afterChild = after.getChildNode(name);
> + if (afterChild == null) {
> + childNodeDeleted(name, beforeChild);
> + } else {
> + beforeChildNodes.add(name);
> + if (!beforeChild.equals(afterChild)) {
> + childNodeChanged(name, beforeChild, afterChild);
> + }
> + }
> + }
> +
> + for (ChildNodeEntry afterChild : after.getChildNodeEntries(0, -1)) {
> + String name = afterChild.getName();
> + if (!beforeChildNodes.contains(name)) {
> + childNodeAdded(name, afterChild.getNode());
> + }
> + }
> + }
> +
> +}
>
> Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/NodeStateDiff.java
> ------------------------------------------------------------------------------
> svn:eol-style = native
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/model/TraversingNodeDiffHandler.java Thu Mar 8 11:35:35 2012
> @@ -16,68 +16,38 @@
> */
> package org.apache.jackrabbit.mk.model;
>
> -import org.apache.jackrabbit.mk.store.RevisionProvider;
> import org.apache.jackrabbit.mk.util.PathUtils;
> +import org.apache.jackrabbit.oak.model.NodeState;
>
> import java.util.Stack;
>
> /**
> *
> */
> -public abstract class TraversingNodeDiffHandler implements NodeDiffHandler {
> +public abstract class TraversingNodeDiffHandler extends NodeStateDiff {
>
> - protected final RevisionProvider store;
> - protected Stack paths = new Stack();
> -
> - public TraversingNodeDiffHandler(RevisionProvider store) {
> - this.store = store;
> - }
> + protected Stack<String> paths = new Stack<String>();
>
> - public void start(Node node1, Node node2) throws Exception {
> - start(node1, node2, "/");
> + public void start(NodeState before, NodeState after) {
> + start(before, after, "/");
> }
>
> - public void start(Node node1, Node node2, String path) throws Exception {
> + public void start(NodeState before, NodeState after, String path) {
> paths.clear();
> paths.push(path);
> - try {
> - node1.diff(node2, this);
> - } catch (RuntimeException e) {
> - Throwable root = e.getCause();
> - if (root != null && root instanceof Exception) {
> - throw (Exception) root;
> - } else {
> - throw e;
> - }
> - }
> - }
> -
> - protected String getCurrentPath() {
> - return (String) paths.peek();
> - }
> -
> - public void propAdded(String propName, String value) {
> - }
> -
> - public void propChanged(String propName, String oldValue, String newValue) {
> + compare(before, after);
> }
>
> - public void propDeleted(String propName, String value) {
> - }
> -
> - public void childNodeAdded(ChildNodeEntry added) {
> - }
> -
> - public void childNodeDeleted(ChildNodeEntry deleted) {
> + protected String getCurrentPath() {
> + return paths.peek();
> }
>
> - public void childNodeChanged(ChildNodeEntry changed, Id newId) {
> - paths.push(PathUtils.concat(getCurrentPath(), changed.getName()));
> - try {
> - store.getNode(changed.getId()).diff(store.getNode(newId), this);
> - } catch (Exception e) {
> - throw new RuntimeException(e);
> - }
> + @Override
> + public void childNodeChanged(
> + String name, NodeState before, NodeState after) {
> + paths.push(PathUtils.concat(getCurrentPath(), name));
> + compare(before, after);
> paths.pop();
> }
> +
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/CopyingGC.java Thu Mar 8 11:35:35 2012
> @@ -175,6 +175,10 @@ public class CopyingGC implements Revisi
> return new StoredNodeAsState(node, this);
> }
>
> + public Id getId(NodeState node) {
> + return ((StoredNodeAsState) node).getId();
> + }
> +
> public StoredNode getNode(Id id) throws NotFoundException, Exception {
> if (running) {
> try {
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java Thu Mar 8 11:35:35 2012
> @@ -247,6 +247,10 @@ public class DefaultRevisionStore implem
> return new StoredNodeAsState(node, this);
> }
>
> + public Id getId(NodeState node) {
> + return ((StoredNodeAsState) node).getId();
> + }
> +
> public StoredNode getNode(Id id) throws NotFoundException, Exception {
> verifyInitialized();
>
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/RevisionProvider.java Thu Mar 8 11:35:35 2012
> @@ -36,6 +36,14 @@ public interface RevisionProvider {
> */
> NodeState getNodeState(StoredNode node);
>
> + /**
> + * Adapts the given {@link NodeState} to the corresponding identifier.
> + *
> + * @param node node state
> + * @return node identifier
> + */
> + Id getId(NodeState node);
> +
> StoredNode getNode(Id id) throws NotFoundException, Exception;
> StoredCommit getCommit(String id) throws NotFoundException, Exception;
> ChildNodeEntriesMap getCNEMap(Id id) throws NotFoundException, Exception;
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java?rev=1298366&r1=1298365&r2=1298366&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java Thu Mar 8 11:35:35 2012
> @@ -20,6 +20,7 @@ import java.util.Collections;
> import java.util.Iterator;
> import java.util.Map;
>
> +import org.apache.jackrabbit.mk.model.Id;
> import org.apache.jackrabbit.mk.model.StoredNode;
> import org.apache.jackrabbit.oak.model.AbstractChildNodeEntry;
> import org.apache.jackrabbit.oak.model.AbstractNodeState;
> @@ -39,6 +40,10 @@ class StoredNodeAsState extends Abstract
> this.provider = provider;
> }
>
> + Id getId() {
> + return node.getId();
> + }
> +
> private static class SimplePropertyState extends AbstractPropertyState {
>
> private final String name;
>
>
Re: svn commit: r1298366 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk:
./ model/ store/
Posted by Stefan Guggisberg <st...@gmail.com>.
On Thu, Apr 5, 2012 at 8:55 AM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Wed, Apr 4, 2012 at 6:24 PM, Stefan Guggisberg
> <st...@gmail.com> wrote:
>> i just realized (sorry, a bit late, i know) that this commit
>> breaks some of the optimizations for large child node lists.
>
> Good point, I didn't consider that case.
>
> I'll refactor the code to restore the ability of the underlying layer
> to optimize this case. I filed OAK-46 to track this.
>
excelllent, thanks!
cheers
stefan
> BR,
>
> Jukka Zitting
Re: svn commit: r1298366 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/mk:
./ model/ store/
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Apr 4, 2012 at 6:24 PM, Stefan Guggisberg
<st...@gmail.com> wrote:
> i just realized (sorry, a bit late, i know) that this commit
> breaks some of the optimizations for large child node lists.
Good point, I didn't consider that case.
I'll refactor the code to restore the ability of the underlying layer
to optimize this case. I filed OAK-46 to track this.
BR,
Jukka Zitting