jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1526767 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: api/AbstractPropertyState.java plugins/segment/Segment.java plugins/segment/SegmentPropertyState.java plugins/segment/Template.java
Date Fri, 27 Sep 2013 01:41:14 GMT
Author: jukka
Date: Fri Sep 27 01:41:14 2013
New Revision: 1526767

URL: http://svn.apache.org/r1526767
Log:
OAK-1031: SegmentMK: Fewer segment lookups

Make SegmentNodeState extend Record and use the built-in segment tracking mechanism.
Clean up the PropertyState equality definition since now also Blob equality is better defined.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/AbstractPropertyState.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentPropertyState.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/api/AbstractPropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/AbstractPropertyState.java?rev=1526767&r1=1526766&r2=1526767&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/AbstractPropertyState.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/AbstractPropertyState.java
Fri Sep 27 01:41:14 2013
@@ -18,12 +18,7 @@
  */
 package org.apache.jackrabbit.oak.api;
 
-import static org.apache.jackrabbit.oak.api.Type.BINARIES;
-import static org.apache.jackrabbit.oak.api.Type.STRING;
-import static org.apache.jackrabbit.oak.api.Type.STRINGS;
-
-import javax.jcr.PropertyType;
-
+import com.google.common.base.Objects;
 import com.google.common.collect.Iterables;
 
 /**
@@ -34,11 +29,53 @@ import com.google.common.collect.Iterabl
 public abstract class AbstractPropertyState implements PropertyState {
 
     /**
-     * Checks whether the given object is equal to this one. Two property
-     * states are considered equal if their names and types match and
-     * their string representation of their values are equal.
-     * Subclasses may override this method with a more efficient
-     * equality check if one is available.
+     * Checks whether the given two property states are equal. They are
+     * considered equal if their names and types match, they have an equal
+     * number of values, and each of the values is equal with the
+     * corresponding value in the other property.
+     *
+     * @param a first property state
+     * @param b second property state
+     * @return {@code true} if the properties are equal, {@code false} otherwise
+     */
+    public static boolean equal(PropertyState a, PropertyState b) {
+        if (Objects.equal(a.getName(), b.getName())
+                && Objects.equal(a.getType(), b.getType())) {
+            Type<?> type = a.getType();
+            if (a.isArray()) {
+                return a.count() == b.count()
+                        && Iterables.elementsEqual(
+                                (Iterable<?>) a.getValue(type),
+                                (Iterable<?>) b.getValue(type));
+            } else {
+                return Objects.equal(a.getValue(type), b.getValue(type));
+            }
+        } else {
+            return false;
+        }
+    }
+
+    public static int hashCode(PropertyState property) {
+        return property.getName().hashCode();
+    }
+
+    public static String toString(PropertyState property) {
+        String name = property.getName();
+        Type<?> type = property.getType();
+        if (type == Type.BINARIES) {
+            return name + " = [" + property.count() + " binaries]";
+        } else if (type == Type.BINARY) {
+            return name + " = {" + property.size() + " bytes}";
+        } else {
+            return name + " = " + property.getValue(type);
+        }
+    }
+
+    /**
+     * Checks whether the given object is equal to this one. See the
+     * {@link #equal(PropertyState, PropertyState)} method for the definition
+     * of property state equality. Subclasses may override this method with
+     * a more efficient equality check if one is available.
      *
      * @param other target of the comparison
      * @return {@code true} if the objects are equal, {@code false} otherwise
@@ -47,23 +84,9 @@ public abstract class AbstractPropertySt
     public boolean equals(Object other) {
         if (this == other) {
             return true;
-        } else if (other instanceof PropertyState) {
-            PropertyState that = (PropertyState) other;
-            if (!getName().equals(that.getName())) {
-                return false;
-            } else if (!getType().equals(that.getType())) {
-                return false;
-            } else if (isArray() && count() != that.count()) {
-                return false;
-            } else if (getType().tag() == PropertyType.BINARY) {
-                return Iterables.elementsEqual(
-                        getValue(BINARIES), that.getValue(BINARIES));
-            } else {
-                return Iterables.elementsEqual(
-                        getValue(STRINGS), that.getValue(STRINGS));
-            }
         } else {
-            return false;
+            return other instanceof PropertyState
+                    && equal(this, (PropertyState) other);
         }
     }
 
@@ -78,20 +101,12 @@ public abstract class AbstractPropertySt
      */
     @Override
     public int hashCode() {
-        return getName().hashCode();
+        return hashCode(this);
     }
 
     @Override
     public String toString() {
-        if (getType() == Type.BINARIES) {
-            return getName() + " = [" + count() + " binaries]"; 
-        } else if (getType() == Type.BINARY) {
-            return getName() + " = {" + size() + " bytes}"; 
-        } else if (isArray()) {
-            return getName() + " = " + getValue(STRINGS);
-        } else {
-            return getName() + " = " + getValue(STRING);
-        }
+        return toString(this);
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java?rev=1526767&r1=1526766&r2=1526767&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
Fri Sep 27 01:41:14 2013
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.oak.plugins.segment;
 
+import static com.google.common.base.Objects.equal;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkPositionIndexes;
 import static org.apache.jackrabbit.oak.plugins.segment.SegmentWriter.BLOCK_SIZE;
@@ -155,11 +156,27 @@ public class Segment {
      * Returns the identified segment.
      *
      * @param uuid segment identifier
+     * @return identified segment
      */
     Segment getSegment(UUID uuid) {
-        return store.readSegment(uuid);
+        if (equal(uuid, this.uuid)) {
+            return this; // optimization for the common case (OAK-1031)
+        } else {
+            return store.readSegment(uuid);
+        }
+    }
+
+    /**
+     * Returns the segment that contains the identified record.
+     *
+     * @param id record identifier
+     * @return segment that contains the identified record
+     */
+    Segment getSegment(RecordId id) {
+        return getSegment(checkNotNull(id).getSegmentId());
     }
 
+
     /**
      * Reads the given number of bytes starting from the given position
      * in this segment.
@@ -197,17 +214,12 @@ public class Segment {
                 | (data.get(pos + 3) & 0xff);
     }
 
-    String readString(int offset) {
-        return strings.get(offset);
+    String readString(RecordId id) {
+        return getSegment(id).readString(id.getOffset());
     }
 
-    String readString(RecordId id) {
-        checkNotNull(id);
-        Segment segment = this;
-        if (!uuid.equals(id.getSegmentId())) {
-            segment = store.readSegment(id.getSegmentId());
-        }
-        return segment.readString(id.getOffset());
+    String readString(int offset) {
+        return strings.get(offset);
     }
 
     private String loadString(int offset) {
@@ -241,17 +253,12 @@ public class Segment {
         }
     }
 
-    Template readTemplate(int offset) {
-        return templates.get(offset);
+    Template readTemplate(RecordId id) {
+        return getSegment(id).readTemplate(id.getOffset());
     }
 
-    Template readTemplate(RecordId id) {
-        checkNotNull(id);
-        Segment segment = this;
-        if (!uuid.equals(id.getSegmentId())) {
-            segment = store.readSegment(id.getSegmentId());
-        }
-        return segment.readTemplate(id.getOffset());
+    Template readTemplate(int offset) {
+        return templates.get(offset);
     }
 
     private Template loadTemplate(int offset) {
@@ -308,6 +315,10 @@ public class Segment {
                 primaryType, mixinTypes, properties, childName);
     }
 
+    long readLength(RecordId id) {
+        return getSegment(id).readLength(id.getOffset());
+    }
+
     long readLength(int offset) {
         return internalReadLength(pos(offset, 1));
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentPropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentPropertyState.java?rev=1526767&r1=1526766&r2=1526767&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentPropertyState.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentPropertyState.java
Fri Sep 27 01:41:14 2013
@@ -30,37 +30,28 @@ import javax.annotation.Nonnull;
 import javax.jcr.PropertyType;
 
 import org.apache.jackrabbit.oak.api.AbstractPropertyState;
+import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.plugins.value.Conversions;
 import org.apache.jackrabbit.oak.plugins.value.Conversions.Converter;
 
-class SegmentPropertyState extends AbstractPropertyState {
+class SegmentPropertyState extends Record implements PropertyState {
 
     private final PropertyTemplate template;
 
-    private final SegmentStore store;
-
-    private final RecordId recordId;
-
     public SegmentPropertyState(
-            PropertyTemplate template, SegmentStore store, RecordId recordId) {
+            Segment segment, RecordId id, PropertyTemplate template) {
+        super(segment, id);
         this.template = checkNotNull(template);
-        this.store = checkNotNull(store);
-        this.recordId = checkNotNull(recordId);
     }
 
-    RecordId getRecordId() {
-        return recordId;
-    }
-
-    private ListRecord getValueList() {
-        RecordId listId = recordId;
+    private ListRecord getValueList(Segment segment) {
+        RecordId listId = getRecordId();
         int size = 1;
-        Segment segment = store.readSegment(recordId.getSegmentId());
         if (isArray()) {
-            size = segment.readInt(recordId.getOffset());
+            size = segment.readInt(getOffset());
             if (size > 0) {
-                listId = segment.readRecordId(recordId.getOffset() + 4);
+                listId = segment.readRecordId(getOffset(4));
             }
         }
         return new ListRecord(segment, listId, size);
@@ -73,8 +64,8 @@ class SegmentPropertyState extends Abstr
 
         Map<String, RecordId> map = newHashMap();
 
-        ListRecord values = getValueList();
-        Segment segment = store.readSegment(recordId.getSegmentId());
+        Segment segment = getSegment();
+        ListRecord values = getValueList(segment);
         for (int i = 0; i < values.size(); i++) {
             RecordId valueId = values.getEntry(i);
             String value = segment.readString(valueId);
@@ -102,8 +93,7 @@ class SegmentPropertyState extends Abstr
     @Override
     public int count() {
         if (isArray()) {
-            Segment segment = store.readSegment(recordId.getSegmentId());
-            return segment.readInt(recordId.getOffset());
+            return getSegment().readInt(getOffset());
         } else {
             return 1;
         }
@@ -153,7 +143,8 @@ class SegmentPropertyState extends Abstr
         checkNotNull(type);
         checkArgument(!type.isArray(), "Type must not be an array type");
 
-        ListRecord values = getValueList();
+        Segment segment = getSegment();
+        ListRecord values = getValueList(segment);
         checkElementIndex(index, values.size());
 
         Type<?> base = getType();
@@ -161,7 +152,6 @@ class SegmentPropertyState extends Abstr
             base = base.getBaseType();
         }
 
-        Segment segment = store.readSegment(recordId.getSegmentId());
         RecordId valueId = values.getEntry(index);
         if (type == Type.BINARY) {
             return (T) new SegmentBlob(segment, valueId);
@@ -202,11 +192,10 @@ class SegmentPropertyState extends Abstr
 
     @Override
     public long size(int index) {
-        ListRecord values = getValueList();
+        Segment segment = getSegment();
+        ListRecord values = getValueList(segment);
         checkElementIndex(index, values.size());
-        RecordId valueId = values.getEntry(0);
-        Segment segment = store.readSegment(valueId.getSegmentId());
-        return segment.readLength(valueId.getOffset());
+        return segment.readLength(values.getEntry(0));
     }
 
 
@@ -219,13 +208,25 @@ class SegmentPropertyState extends Abstr
             return true;
         } else if (object instanceof SegmentPropertyState) {
             SegmentPropertyState that = (SegmentPropertyState) object;
-            if (recordId.equals(that.recordId)
-                    && template.equals(that.template)) {
+            if (!template.equals(that.template)) {
+                return false;
+            } else if (getRecordId().equals(that.getRecordId())) {
                 return true;
             }
         }
         // fall back to default equality check in AbstractPropertyState
-        return super.equals(object);
+        return object instanceof PropertyState
+                && AbstractPropertyState.equal(this, (PropertyState) object);
+    }
+
+    @Override
+    public int hashCode() {
+        return AbstractPropertyState.hashCode(this);
+    }
+
+    @Override
+    public String toString() {
+        return AbstractPropertyState.toString(this);
     }
 
 }

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=1526767&r1=1526766&r2=1526767&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
Fri Sep 27 01:41:14 2013
@@ -238,7 +238,7 @@ public class Template {
         offset += index * RECORD_ID_BYTES;
         Segment segment = store.readSegment(recordId.getSegmentId());
         return new SegmentPropertyState(
-                properties[index], store, segment.readRecordId(offset));
+                segment, segment.readRecordId(offset), properties[index]);
     }
 
     public Iterable<PropertyState> getProperties(
@@ -259,7 +259,7 @@ public class Template {
         for (int i = 0; i < properties.length; i++) {
             RecordId propertyId = segment.readRecordId(offset);
             list.add(new SegmentPropertyState(
-                    properties[i], store, propertyId));
+                    segment, propertyId, properties[i]));
             offset += RECORD_ID_BYTES;
         }
         return list;
@@ -592,7 +592,7 @@ public class Template {
         // Other properties
         for (int i = 0; i < properties.length; i++) {
             if (!diff.propertyAdded(new SegmentPropertyState(
-                    properties[i], store, segment.readRecordId(offset)))) {
+                    segment, segment.readRecordId(offset), properties[i]))) {
                 return false;
             }
             offset += RECORD_ID_BYTES;



Mime
View raw message