Author: mreutegg
Date: Wed Mar 5 20:28:47 2014
New Revision: 1574648
URL: http://svn.apache.org/r1574648
Log:
OAK-1429: Slow event listeners do not scale as expected
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java?rev=1574648&r1=1574647&r2=1574648&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
Wed Mar 5 20:28:47 2014
@@ -214,7 +214,7 @@ public class DocumentMK implements Micro
if (maxChildNodes-- <= 0) {
break;
}
- String name = PathUtils.getName(c.children.get((int) i));
+ String name = c.children.get((int) i);
json.key(name).object().endObject();
}
if (c.hasMore) {
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java?rev=1574648&r1=1574647&r2=1574648&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java
Wed Mar 5 20:28:47 2014
@@ -381,28 +381,25 @@ class DocumentNodeState extends Abstract
}
switch (r) {
case '+': {
- String path = t.readString();
+ String name = t.readString();
t.read(':');
t.read('{');
while (t.read() != '}') {
// skip properties
}
- String name = PathUtils.getName(path);
continueComparison = diff.childNodeAdded(name,
getChildNode(name));
break;
}
case '-': {
- String path = t.readString();
- String name = PathUtils.getName(path);
+ String name = t.readString();
continueComparison = diff.childNodeDeleted(name,
base.getChildNode(name));
break;
}
case '^': {
- String path = t.readString();
+ String name = t.readString();
t.read(':');
if (t.matches('{')) {
t.read('}');
- String name = PathUtils.getName(path);
continueComparison = diff.childNodeChanged(name,
base.getChildNode(name), getChildNode(name));
} else if (t.matches('[')) {
@@ -416,21 +413,6 @@ class DocumentNodeState extends Abstract
}
break;
}
- case '>': {
- String from = t.readString();
- t.read(':');
- String to = t.readString();
- String fromName = PathUtils.getName(from);
- continueComparison = diff.childNodeDeleted(
- fromName, base.getChildNode(fromName));
- if (!continueComparison) {
- break;
- }
- String toName = PathUtils.getName(to);
- continueComparison = diff.childNodeAdded(
- toName, getChildNode(toName));
- break;
- }
default:
throw new IllegalArgumentException("jsonDiff: illegal
token '"
+ t.getToken() + "' at pos: " + t.getLastPos() + '
' + jsonDiff);
@@ -478,6 +460,9 @@ class DocumentNodeState extends Abstract
*/
public static class Children implements CacheValue {
+ /**
+ * Ascending sorted list of names of child nodes.
+ */
final ArrayList<String> children = new ArrayList<String>();
boolean hasMore;
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1574648&r1=1574647&r2=1574648&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Wed Mar 5 20:28:47 2014
@@ -56,6 +56,8 @@ import com.google.common.collect.Sets;
import org.apache.jackrabbit.mk.api.MicroKernelException;
import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.commons.json.JsopReader;
+import org.apache.jackrabbit.oak.commons.json.JsopTokenizer;
import org.apache.jackrabbit.oak.spi.blob.BlobStore;
import org.apache.jackrabbit.oak.commons.json.JsopStream;
import org.apache.jackrabbit.oak.commons.json.JsopWriter;
@@ -665,7 +667,7 @@ public final class DocumentNodeStore
}
if (c.children.size() < limit) {
// add to children until limit is reached
- c.children.add(p);
+ c.children.add(Utils.unshareString(PathUtils.getName(p)));
} else {
// enough collected and we know there are more
c.hasMore = true;
@@ -785,7 +787,8 @@ public final class DocumentNodeStore
new Function<String, DocumentNodeState>() {
@Override
public DocumentNodeState apply(String input) {
- return getNode(input, readRevision);
+ String p = PathUtils.concat(parent.getPath(), input);
+ return getNode(p, readRevision);
}
});
}
@@ -832,10 +835,9 @@ public final class DocumentNodeStore
if (isNew) {
CacheValue key = childNodeCacheKey(path, rev, null);
DocumentNodeState.Children c = new DocumentNodeState.Children();
- Set<String> set = Sets.newTreeSet(added);
- set.removeAll(removed);
+ Set<String> set = Sets.newTreeSet();
for (String p : added) {
- set.add(Utils.unshareString(p));
+ set.add(Utils.unshareString(PathUtils.getName(p)));
}
c.children.addAll(set);
nodeChildrenCache.put(key, c);
@@ -844,13 +846,13 @@ public final class DocumentNodeStore
PathRev key = diffCacheKey(path, before, rev);
JsopWriter w = new JsopStream();
for (String p : added) {
- w.tag('+').key(p).object().endObject().newline();
+
w.tag('+').key(PathUtils.getName(p)).object().endObject().newline();
}
for (String p : removed) {
- w.tag('-').value(p).newline();
+ w.tag('-').value(PathUtils.getName(p)).newline();
}
for (String p : changed) {
- w.tag('^').key(p).object().endObject().newline();
+
w.tag('^').key(PathUtils.getName(p)).object().endObject().newline();
}
diffCache.put(key, new StringValue(w.toString()));
}
@@ -1148,12 +1150,35 @@ public final class DocumentNodeStore
try {
JsopWriter writer = new JsopStream();
diffProperties(from, to, writer);
- return writer.toString() + diffCache.get(key, new
Callable<StringValue>() {
+ String compactDiff = diffCache.get(key, new
Callable<StringValue>() {
@Override
public StringValue call() throws Exception {
return new StringValue(diffImpl(from, to));
}
- });
+ }).toString();
+ JsopTokenizer t = new JsopTokenizer(compactDiff);
+ int r;
+ do {
+ r = t.read();
+ switch (r) {
+ case '+':
+ case '^': {
+ String name = t.readString();
+ t.read(':');
+ t.read('{');
+ t.read('}');
+ writer.tag((char) r).key(PathUtils.concat(path, name));
+ writer.object().endObject().newline();
+ break;
+ }
+ case '-': {
+ String name = t.readString();
+ writer.tag('-').value(PathUtils.concat(path, name));
+ writer.newline();
+ }
+ }
+ } while (r != JsopReader.END);
+ return writer.toString();
} catch (ExecutionException e) {
if (e.getCause() instanceof MicroKernelException) {
throw (MicroKernelException) e.getCause();
@@ -1414,7 +1439,6 @@ public final class DocumentNodeStore
private String diffImpl(DocumentNodeState from, DocumentNodeState to)
throws MicroKernelException {
JsopWriter w = new JsopStream();
- diffProperties(from, to, w);
// TODO this does not work well for large child node lists
// use a document store index instead
int max = MANY_CHILDREN_THRESHOLD;
@@ -1422,8 +1446,8 @@ public final class DocumentNodeStore
fromChildren = getChildren(from, null, max);
toChildren = getChildren(to, null, max);
if (!fromChildren.hasMore && !toChildren.hasMore) {
- diffFewChildren(w, fromChildren, from.getLastRevision(),
- toChildren, to.getLastRevision());
+ diffFewChildren(w, from.getPath(), fromChildren,
+ from.getLastRevision(), toChildren, to.getLastRevision());
} else {
if (FAST_DIFF) {
diffManyChildren(w, from.getPath(),
@@ -1432,8 +1456,8 @@ public final class DocumentNodeStore
max = Integer.MAX_VALUE;
fromChildren = getChildren(from, null, max);
toChildren = getChildren(to, null, max);
- diffFewChildren(w, fromChildren, from.getLastRevision(),
- toChildren, to.getLastRevision());
+ diffFewChildren(w, from.getPath(), fromChildren,
+ from.getLastRevision(), toChildren,
to.getLastRevision());
}
}
return w.toString();
@@ -1463,23 +1487,24 @@ public final class DocumentNodeStore
for (String p : paths) {
DocumentNodeState fromNode = getNode(p, fromRev);
DocumentNodeState toNode = getNode(p, toRev);
+ String name = PathUtils.getName(p);
if (fromNode != null) {
// exists in fromRev
if (toNode != null) {
// exists in both revisions
// check if different
if
(!fromNode.getLastRevision().equals(toNode.getLastRevision())) {
- w.tag('^').key(p).object().endObject().newline();
+ w.tag('^').key(name).object().endObject().newline();
}
} else {
// does not exist in toRev -> was removed
- w.tag('-').value(p).newline();
+ w.tag('-').value(name).newline();
}
} else {
// does not exist in fromRev
if (toNode != null) {
// exists in toRev
- w.tag('+').key(p).object().endObject().newline();
+ w.tag('+').key(name).object().endObject().newline();
} else {
// does not exist in either revisions
// -> do nothing
@@ -1503,21 +1528,22 @@ public final class DocumentNodeStore
}
}
- private void diffFewChildren(JsopWriter w, DocumentNodeState.Children
fromChildren, Revision fromRev, DocumentNodeState.Children toChildren, Revision
toRev) {
+ private void diffFewChildren(JsopWriter w, String parentPath,
DocumentNodeState.Children fromChildren, Revision fromRev,
DocumentNodeState.Children toChildren, Revision toRev) {
Set<String> childrenSet = Sets.newHashSet(toChildren.children);
for (String n : fromChildren.children) {
if (!childrenSet.contains(n)) {
w.tag('-').value(n).newline();
} else {
- DocumentNodeState n1 = getNode(n, fromRev);
- DocumentNodeState n2 = getNode(n, toRev);
+ String path = PathUtils.concat(parentPath, n);
+ DocumentNodeState n1 = getNode(path, fromRev);
+ DocumentNodeState n2 = getNode(path, toRev);
// this is not fully correct:
// a change is detected if the node changed recently,
// even if the revisions are well in the past
// if this is a problem it would need to be changed
- checkNotNull(n1, "Node at [%s] not found for fromRev [%s]", n,
fromRev);
- checkNotNull(n2, "Node at [%s] not found for toRev [%s]", n,
toRev);
- if (!n1.getId().equals(n2.getId())) {
+ checkNotNull(n1, "Node at [%s] not found for fromRev [%s]",
path, fromRev);
+ checkNotNull(n2, "Node at [%s] not found for toRev [%s]",
path, toRev);
+ if (!n1.getLastRevision().equals(n2.getLastRevision())) {
w.tag('^').key(n).object().endObject().newline();
}
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java?rev=1574648&r1=1574647&r2=1574648&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java
Wed Mar 5 20:28:47 2014
@@ -252,11 +252,11 @@ public class SimpleTest {
DocumentNodeState n = ns.getNode("/", Revision.fromString(r0));
assertNotNull(n);
Children c = ns.getChildren(n, null, Integer.MAX_VALUE);
- assertEquals("[/test]", c.toString());
+ assertEquals("[test]", c.toString());
n = ns.getNode("/test", Revision.fromString(r1));
assertNotNull(n);
c = ns.getChildren(n, null, Integer.MAX_VALUE);
- assertEquals("[/test/a, /test/b]", c.toString());
+ assertEquals("[a, b]", c.toString());
rev = mk.commit("", "^\"/test\":1", null, null);
test = mk.getNodes("/", rev, 0, 0, Integer.MAX_VALUE, null);
