You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2013/12/18 19:16:39 UTC

svn commit: r1552049 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment: MapRecord.java Template.java

Author: jukka
Date: Wed Dec 18 18:16:38 2013
New Revision: 1552049

URL: http://svn.apache.org/r1552049
Log:
OAK-1273: Reduce overhead when handling many parallel property indices

Simplify content diff stack traces by dropping the MapDiff wrapper to NodeStateDiff

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java?rev=1552049&r1=1552048&r2=1552049&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java Wed Dec 18 18:16:38 2013
@@ -28,6 +28,8 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
+
 import com.google.common.base.Objects;
 import com.google.common.collect.ComparisonChain;
 
@@ -354,22 +356,7 @@ class MapRecord extends Record {
         return Arrays.asList(entries);
     }
 
-    boolean compareAgainstEmptyMap(MapDiff diff) {
-        for (MapEntry entry : getEntries()) {
-            if (!diff.entryAdded(entry)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    interface MapDiff {
-        boolean entryAdded(MapEntry after);
-        boolean entryChanged(MapEntry before, MapEntry after);
-        boolean entryDeleted(MapEntry before);
-    }
-
-    boolean compare(MapRecord before, MapDiff diff) {
+    boolean compare(MapRecord before, NodeStateDiff diff) {
         Segment beforeSegment = before.getSegment();
         int beforeHead = beforeSegment.readInt(before.getOffset(0));
 
@@ -385,9 +372,10 @@ class MapRecord extends Record {
                 RecordId afterValue = afterSegment.readRecordId(after.getOffset(8, 1));
                 RecordId beforeValue = before.getValue(hash, key);
                 String name = beforeSegment.readString(key);
-                return diff.entryChanged(
-                        new MapEntry(beforeSegment, name, key, beforeValue),
-                        new MapEntry(afterSegment, name, key, afterValue));
+                return diff.childNodeChanged(
+                        name,
+                        new SegmentNodeState(beforeSegment, beforeValue),
+                        new SegmentNodeState(afterSegment, afterValue));
             } else if (isDiff(beforeHead)) {
                 RecordId beforeBase =
                         beforeSegment.readRecordId(before.getOffset(8, 2));
@@ -402,19 +390,22 @@ class MapRecord extends Record {
 
                     if (beforeKey.equals(afterKey)) {
                         String name = beforeSegment.readString(beforeKey);
-                        return diff.entryChanged(
-                                new MapEntry(beforeSegment, name, beforeKey, beforeValue),
-                                new MapEntry(afterSegment, name, afterKey, afterValue));
+                        return diff.childNodeChanged(
+                                name,
+                                new SegmentNodeState(beforeSegment, beforeValue),
+                                new SegmentNodeState(afterSegment, afterValue));
                     } else {
                         String beforeName = beforeSegment.readString(beforeKey);
                         String afterName = afterSegment.readString(afterKey);
-                        return diff.entryChanged(
-                                new MapEntry(beforeSegment, beforeName, beforeKey, beforeValue),
-                                new MapEntry(afterSegment, beforeName, beforeKey, after.getValue(beforeHash, beforeKey)))
+                        return diff.childNodeChanged(
+                                beforeName,
+                                new SegmentNodeState(beforeSegment, beforeValue),
+                                new SegmentNodeState(afterSegment, after.getValue(beforeHash, beforeKey)))
                                &&
-                               diff.entryChanged(
-                                new MapEntry(beforeSegment, afterName, afterKey, before.getValue(afterHash, afterKey)),
-                                new MapEntry(afterSegment, afterName, afterKey, afterValue));
+                               diff.childNodeChanged(
+                                afterName,
+                                new SegmentNodeState(beforeSegment, before.getValue(afterHash, afterKey)),
+                                new SegmentNodeState(afterSegment, afterValue));
                     }
                 }
             }
@@ -430,18 +421,24 @@ class MapRecord extends Record {
         while (beforeEntry != null || afterEntry != null) {
             int d = compare(beforeEntry, afterEntry);
             if (d < 0) {
-                if (!diff.entryDeleted(beforeEntry)) {
+                if (!diff.childNodeDeleted(
+                        beforeEntry.getName(), beforeEntry.getNodeState())) {
                     return false;
                 }
                 beforeEntry = nextOrNull(beforeEntries);
             } else if (d == 0) {
-                if (!diff.entryChanged(beforeEntry, afterEntry)) {
+                if (!beforeEntry.getValue().equals(afterEntry.getValue())
+                        && !diff.childNodeChanged(
+                                beforeEntry.getName(),
+                                beforeEntry.getNodeState(),
+                                afterEntry.getNodeState())) {
                     return false;
                 }
                 beforeEntry = nextOrNull(beforeEntries);
                 afterEntry = nextOrNull(afterEntries);
             } else {
-                if (!diff.entryAdded(afterEntry)) {
+                if (!diff.childNodeAdded(
+                        afterEntry.getName(), afterEntry.getNodeState())) {
                     return false;
                 }
                 afterEntry = nextOrNull(afterEntries);
@@ -480,7 +477,7 @@ class MapRecord extends Record {
      * with the same hash prefixes.
      */
     private static boolean compareBranch(
-            MapRecord before, MapRecord after, MapDiff diff) {
+            MapRecord before, MapRecord after, NodeStateDiff diff) {
         MapRecord[] beforeBuckets = before.getBuckets();
         MapRecord[] afterBuckets = after.getBuckets();
         for (int i = 0; i < BUCKETS_PER_LEVEL; i++) {
@@ -490,7 +487,8 @@ class MapRecord extends Record {
                 // before bucket is empty, so all after entries were added
                 MapRecord bucket = afterBuckets[i];
                 for (MapEntry entry : bucket.getEntries()) {
-                    if (!diff.entryAdded(entry)) {
+                    if (!diff.childNodeAdded(
+                            entry.getName(), entry.getNodeState())) {
                         return false;
                     }
                 }
@@ -498,7 +496,8 @@ class MapRecord extends Record {
                 // after bucket is empty, so all before entries were deleted
                 MapRecord bucket = beforeBuckets[i];
                 for (MapEntry entry : bucket.getEntries()) {
-                    if (!diff.entryDeleted(entry)) {
+                    if (!diff.childNodeDeleted(
+                            entry.getName(), entry.getNodeState())) {
                         return false;
                     }
                 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java?rev=1552049&r1=1552048&r2=1552049&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java Wed Dec 18 18:16:38 2013
@@ -36,7 +36,6 @@ import com.google.common.collect.Lists;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
-import org.apache.jackrabbit.oak.plugins.segment.MapRecord.MapDiff;
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
@@ -393,25 +392,7 @@ public class Template {
             // TODO: Leverage the HAMT data structure for the comparison
             MapRecord afterMap = getChildNodeMap(afterSegment, afterId);
             MapRecord beforeMap = beforeTemplate.getChildNodeMap(beforeSegment, beforeId);
-            return afterMap.compare(beforeMap, new MapDiff() {
-                @Override
-                public boolean entryAdded(MapEntry after) {
-                    return diff.childNodeAdded(
-                            after.getName(), after.getNodeState());
-                }
-                @Override
-                public boolean entryChanged(MapEntry before, MapEntry after) {
-                    SegmentNodeState b = before.getNodeState();
-                    SegmentNodeState a = after.getNodeState();
-                    return fastEquals(a, b)
-                            || diff.childNodeChanged(before.getName(), b, a);
-                }
-                @Override
-                public boolean entryDeleted(MapEntry before) {
-                    return diff.childNodeDeleted(
-                            before.getName(), before.getNodeState());
-                }
-            });
+            return afterMap.compare(beforeMap, diff);
         }
 
         return true;
@@ -437,21 +418,12 @@ public class Template {
         if (childName == MANY_CHILD_NODES) {
             RecordId childNodesId = segment.readRecordId(offset);
             MapRecord children = segment.readMap(childNodesId);
-            children.compareAgainstEmptyMap(new MapDiff() {
-                @Override
-                public boolean entryAdded(MapEntry after) {
-                    return diff.childNodeAdded(
-                            after.getName(), after.getNodeState());
-                }
-                @Override
-                public boolean entryChanged(MapEntry before, MapEntry after) {
-                    throw new IllegalStateException();
-                }
-                @Override
-                public boolean entryDeleted(MapEntry before) {
-                    throw new IllegalStateException();
+            for (MapEntry entry : children.getEntries()) {
+                if (!diff.childNodeAdded(
+                        entry.getName(), entry.getNodeState())) {
+                    return false;
                 }
-            });
+            }
             offset += RECORD_ID_BYTES;
         } else if (childName != ZERO_CHILD_NODES) {
             RecordId childNodeId = segment.readRecordId(offset);