jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r1434720 - in /jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk: model/tree/AbstractNodeState.java model/tree/AbstractPropertyState.java model/tree/DiffBuilder.java store/StoredNodeAsState.java
Date Thu, 17 Jan 2013 15:25:31 GMT
Author: thomasm
Date: Thu Jan 17 15:25:30 2013
New Revision: 1434720

URL: http://svn.apache.org/viewvc?rev=1434720&view=rev
Log:
OAK-567 DiffBuilder performance problem

Modified:
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractNodeState.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractPropertyState.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractNodeState.java?rev=1434720&r1=1434719&r2=1434720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractNodeState.java
(original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractNodeState.java
Thu Jan 17 15:25:30 2013
@@ -113,16 +113,19 @@ public abstract class AbstractNodeState 
     }
 
     /**
-     * Returns a hash code that's compatible with how the
-     * {@link #equals(Object)} method is implemented. The current
-     * implementation simply returns zero for everything since
-     * {@link NodeState} instances are not intended for use as hash keys.
+     * Returns the hash code. This method is relatively expensive, and the
+     * returned value is not very distinct, as {@link NodeState} instances are
+     * not intended for use as hash keys.
      *
-     * @return hash code
+     * @return the hash code
      */
     @Override
     public int hashCode() {
-        return 0;
+        int hash = (int) getChildNodeCount();
+        for (PropertyState p : getProperties()) {
+            hash ^= p.hashCode();
+        }
+        return hash;
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractPropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractPropertyState.java?rev=1434720&r1=1434719&r2=1434720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractPropertyState.java
(original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/AbstractPropertyState.java
Thu Jan 17 15:25:30 2013
@@ -48,16 +48,15 @@ public abstract class AbstractPropertySt
 
     /**
      * Returns a hash code that's compatible with how the
-     * {@link #equals(Object)} method is implemented. The current
-     * implementation simply returns the hash code of the property name
-     * since {@link PropertyState} instances are not intended for use as
-     * hash keys.
+     * {@link #equals(Object)} method is implemented. The current implementation
+     * simply returns the hash code of the property name and value.
+     * {@link PropertyState} instances are not intended for use as hash keys.
      *
      * @return hash code
      */
     @Override
     public int hashCode() {
-        return getName().hashCode();
+        return getName().hashCode() ^ getEncodedValue().hashCode();
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java?rev=1434720&r1=1434719&r2=1434720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
(original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/model/tree/DiffBuilder.java
Thu Jan 17 15:25:30 2013
@@ -46,9 +46,15 @@ public class DiffBuilder {
 
     public String build() throws Exception {
         final JsopBuilder buff = new JsopBuilder();
-        // maps (key: id of target node, value: path/to/target)
+
+        // maps (key: the target node, value: the path to the target)
         // for tracking added/removed nodes; this allows us
         // to detect 'move' operations
+
+        // TODO performance problem: this class uses NodeState as a hash key,
+        // which is not recommended because the hashCode and equals methods
+        // of those classes are slow
+
         final HashMap<NodeState, String> addedNodes = new HashMap<NodeState, String>();
         final HashMap<NodeState, String> removedNodes = new HashMap<NodeState, String>();
 

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java?rev=1434720&r1=1434719&r2=1434720&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
(original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/StoredNodeAsState.java
Thu Jan 17 15:25:30 2013
@@ -32,9 +32,9 @@ import java.util.Map;
 
 class StoredNodeAsState extends AbstractNodeState {
 
-    private final StoredNode node;
+    final StoredNode node;
 
-    private final RevisionProvider provider;
+    final RevisionProvider provider;
 
     public StoredNodeAsState(StoredNode node, RevisionProvider provider) {
         this.node = node;
@@ -160,7 +160,7 @@ class StoredNodeAsState extends Abstract
         }
     }
 
-    private ChildNode getChildNodeEntry(
+    ChildNode getChildNodeEntry(
             final ChildNodeEntry entry) {
         return new AbstractChildNode() {
             @Override
@@ -183,10 +183,7 @@ class StoredNodeAsState extends Abstract
     public boolean equals(Object that) {
         if (that instanceof StoredNodeAsState) {
             StoredNodeAsState other = (StoredNodeAsState) that;
-            if (provider == other.provider
-                    && node.getId().equals(other.node.getId())) {
-                return true;
-            }
+            return node.getId().equals(other.node.getId());
         }
         return super.equals(that);
     }



Mime
View raw message