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/11/25 14:54:38 UTC

svn commit: r1545283 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/api/ oak-core/src/main/java/org/apache/jackrabbit/oak/query/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/ oak-jcr/src/main/java/org/apach...

Author: jukka
Date: Mon Nov 25 13:54:37 2013
New Revision: 1545283

URL: http://svn.apache.org/r1545283
Log:
OAK-1130: Performance issue with MutableTree

Avoid extra path lookups in QueryResultImpl

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/ResultRow.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ResultRowImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryResultImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/RowImpl.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/ResultRow.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/ResultRow.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/ResultRow.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/ResultRow.java Mon Nov 25 13:54:37 2013
@@ -21,7 +21,7 @@ package org.apache.jackrabbit.oak.api;
  * A query result row.
  */
 public interface ResultRow {
-    
+
     /**
      * The path, assuming there is only one selector.
      * 
@@ -41,6 +41,16 @@ public interface ResultRow {
     String getPath(String selectorName);
 
     /**
+     * The tree for the given selector name.
+     *
+     * @param selectorName the selector name (null if there is only one selector)
+     * @return the tree
+     * @throws IllegalArgumentException if the selector was not found,
+     *      or if there are multiple selectors but the passed selectorName is null
+     */
+    Tree getTree(String selectorName);
+
+    /**
      * The property value.
      * 
      * @param columnName the column name

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Tree.java Mon Nov 25 13:54:37 2013
@@ -334,6 +334,11 @@ public interface Tree {
     void removeProperty(@Nonnull String name);
 
     /**
+     * Empty array of trees.
+     */
+    Tree[] EMPTY_ARRAY = new Tree[0];
+
+    /**
      * Mapping from a Tree instance to its name.
      */
     Function<Tree, String> GET_NAME =

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java Mon Nov 25 13:54:37 2013
@@ -372,7 +372,7 @@ public class QueryImpl implements Query 
             String plan = getPlan();
             columns = new ColumnImpl[] { new ColumnImpl("explain", "plan", "plan")};
             ResultRowImpl r = new ResultRowImpl(this,
-                    new String[0], 
+                    Tree.EMPTY_ARRAY,
                     new PropertyValue[] { PropertyValues.newString(plan)},
                     null);
             return Arrays.asList(r).iterator();
@@ -396,7 +396,7 @@ public class QueryImpl implements Query 
             };
             ArrayList<ResultRowImpl> list = new ArrayList<ResultRowImpl>();
             ResultRowImpl r = new ResultRowImpl(this,
-                    new String[0],
+                    Tree.EMPTY_ARRAY,
                     new PropertyValue[] {
                             PropertyValues.newString("query"),
                             PropertyValues.newLong(rowIt.getReadCount())
@@ -405,7 +405,7 @@ public class QueryImpl implements Query 
             list.add(r);
             for (SelectorImpl selector : selectors) {
                 r = new ResultRowImpl(this,
-                        new String[0],
+                        Tree.EMPTY_ARRAY,
                         new PropertyValue[] {
                                 PropertyValues.newString(selector.getSelectorName()),
                                 PropertyValues.newLong(selector.getScanCount()),
@@ -516,10 +516,10 @@ public class QueryImpl implements Query 
 
     ResultRowImpl currentRow() {
         int selectorCount = selectors.size();
-        String[] paths = new String[selectorCount];
+        Tree[] trees = new Tree[selectorCount];
         for (int i = 0; i < selectorCount; i++) {
             SelectorImpl s = selectors.get(i);
-            paths[i] = s.currentPath();
+            trees[i] = s.currentTree();
         }
         int columnCount = columns.length;
         PropertyValue[] values = new PropertyValue[columnCount];
@@ -537,7 +537,7 @@ public class QueryImpl implements Query 
                 orderValues[i] = orderings[i].getOperand().currentProperty();
             }
         }
-        return new ResultRowImpl(this, paths, values, orderValues);
+        return new ResultRowImpl(this, trees, values, orderValues);
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ResultRowImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ResultRowImpl.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ResultRowImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ResultRowImpl.java Mon Nov 25 13:54:37 2013
@@ -22,6 +22,7 @@ import java.util.Comparator;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.ResultRow;
+import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.query.ast.ColumnImpl;
 import org.apache.jackrabbit.oak.query.ast.OrderingImpl;
 import org.apache.jackrabbit.oak.query.fulltext.SimpleExcerptProvider;
@@ -33,13 +34,13 @@ import org.apache.jackrabbit.oak.spi.que
 public class ResultRowImpl implements ResultRow {
 
     private final Query query;
-    private final String[] paths;
+    private final Tree[] trees;
     private final PropertyValue[] values;
     private final PropertyValue[] orderValues;
 
-    ResultRowImpl(Query query, String[] paths, PropertyValue[] values, PropertyValue[] orderValues) {
+    ResultRowImpl(Query query, Tree[] trees, PropertyValue[] values, PropertyValue[] orderValues) {
         this.query = query;
-        this.paths = paths;
+        this.trees = trees;
         this.values = values;
         this.orderValues = orderValues;
     }
@@ -55,19 +56,29 @@ public class ResultRowImpl implements Re
 
     @Override
     public String getPath(String selectorName) {
+        Tree tree = getTree(selectorName);
+        if (tree != null) {
+            return tree.getPath();
+        } else {
+            return null;
+        }
+    }
+
+    @Override
+    public Tree getTree(String selectorName) {
         if (selectorName == null) {
-            if (paths.length > 1) {
+            if (trees.length > 1) {
                 throw new IllegalArgumentException("More than one selector");
-            } else if (paths.length == 0) {
+            } else if (trees.length == 0) {
                 throw new IllegalArgumentException("This query does not have a selector");
             }
-            return paths[0];
+            return trees[0];
         }
         int index = query.getSelectorIndex(selectorName);
-        if (paths == null || index >= paths.length) {
+        if (trees == null || index >= trees.length) {
             return null;
         }
-        return paths[index];
+        return trees[index];
     }
 
     @Override
@@ -125,7 +136,7 @@ public class ResultRowImpl implements Re
     @Override
     public int hashCode() {
         int result = 1;
-        result = 31 * result + Arrays.hashCode(paths);
+        result = 31 * result + Arrays.hashCode(getPaths());
         result = 31 * result + Arrays.hashCode(values);
         return result;
     }
@@ -140,7 +151,7 @@ public class ResultRowImpl implements Re
             return false;
         }
         ResultRowImpl other = (ResultRowImpl) obj;
-        if (!Arrays.equals(paths, other.paths)) {
+        if (!Arrays.equals(getPaths(), other.getPaths())) {
             return false;
         } else if (!Arrays.equals(values, other.values)) {
             return false;
@@ -148,6 +159,18 @@ public class ResultRowImpl implements Re
         return true;
     }
 
+    private String[] getPaths() {
+        String[] paths = new String[trees.length];
+        for (int i = 0; i < trees.length; i++) {
+            if (trees[i] != null) {
+                paths[i] = trees[i].getPath();
+            } else {
+                paths[i] = null;
+            }
+        }
+        return paths;
+    }
+
     public static Comparator<ResultRowImpl> getComparator(
             final OrderingImpl[] orderings) {
         if (orderings == null) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java Mon Nov 25 13:54:37 2013
@@ -219,7 +219,7 @@ public class UnionQueryImpl implements Q
             String plan = getPlan();
             columns = new ColumnImpl[] { new ColumnImpl("explain", "plan", "plan")};
             ResultRowImpl r = new ResultRowImpl(this,
-                    new String[0], 
+                    Tree.EMPTY_ARRAY,
                     new PropertyValue[] { PropertyValues.newString(plan)},
                     null);
             return Arrays.asList(r).iterator();

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java Mon Nov 25 13:54:37 2013
@@ -96,7 +96,7 @@ public class NodeDelegate extends ItemDe
     /** The underlying {@link org.apache.jackrabbit.oak.api.Tree} of this node. */
     private final Tree tree;
 
-    protected NodeDelegate(SessionDelegate sessionDelegate, Tree tree) {
+    public NodeDelegate(SessionDelegate sessionDelegate, Tree tree) {
         super(sessionDelegate);
         this.tree = tree;
     }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryResultImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryResultImpl.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryResultImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryResultImpl.java Mon Nov 25 13:54:37 2013
@@ -34,7 +34,7 @@ import org.apache.jackrabbit.commons.ite
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.Result;
 import org.apache.jackrabbit.oak.api.ResultRow;
-import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.jcr.session.NodeImpl;
 import org.apache.jackrabbit.oak.jcr.session.SessionContext;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
@@ -69,16 +69,11 @@ public class QueryResultImpl implements 
 
     private final SessionContext sessionContext;
     private final SessionDelegate sessionDelegate;
-    private final String pathFilter;
-    
+
     public QueryResultImpl(SessionContext sessionContext, Result result) {
         this.sessionContext = sessionContext;
         this.sessionDelegate = sessionContext.getSessionDelegate();
         this.result = result;
-
-        // TODO the path currently contains the workspace name
-        // TODO filter in oak-core once we support workspaces there
-        pathFilter = "/";
     }
 
     @Override
@@ -91,18 +86,6 @@ public class QueryResultImpl implements 
         return result.getSelectorNames();
     }
 
-    boolean includeRow(String path) {
-        if (path == null) {
-            // a row without path (explain,...)
-            return true;
-        }
-        if (PathUtils.isAncestor(pathFilter, path) || pathFilter.equals(path)) {
-            // a row within this workspace
-            return true;
-        }
-        return false;
-    }
-
     @Override
     public RowIterator getRows() throws RepositoryException {
         Iterator<RowImpl> rowIterator = new Iterator<RowImpl>() {
@@ -122,21 +105,11 @@ public class QueryResultImpl implements 
             }
 
             private void fetch() {
-                current = null;
-                while (it.hasNext()) {
-                    ResultRow r = it.next();
-                    boolean include = true;
-                    for (String s : getSelectorNames()) {
-                        String path = r.getPath(s);
-                        if (!includeRow(path)) {
-                            include = false;
-                            break;
-                        }
-                    }
-                    if (include) {
-                        current = new RowImpl(QueryResultImpl.this, r, pathSelector);
-                        return;
-                    }
+                if (it.hasNext()) {
+                    current = new RowImpl(
+                            QueryResultImpl.this, it.next(), pathSelector);
+                } else {
+                    current = null;
                 }
             }
 
@@ -174,19 +147,13 @@ public class QueryResultImpl implements 
     }
 
     @CheckForNull
-    NodeImpl<? extends NodeDelegate> getNode(String path) throws RepositoryException {
-        if (path == null) {
-            return null;
-        }
-        NodeDelegate d = sessionDelegate.getNode(path);
-        return d == null ? null : NodeImpl.createNode(d, sessionContext);
-    }
-
-    String getLocalPath(String path) {
-        if (path == null) {
+    NodeImpl<? extends NodeDelegate> getNode(Tree tree) throws RepositoryException {
+        if (tree != null && tree.exists()) {
+            NodeDelegate node = new NodeDelegate(sessionDelegate, tree);
+            return NodeImpl.createNode(node, sessionContext);
+        } else {
             return null;
         }
-        return PathUtils.concat("/", PathUtils.relativize(pathFilter, path));
     }
 
     @Override
@@ -214,13 +181,14 @@ public class QueryResultImpl implements 
                 current = null;
                 while (it.hasNext()) {
                     ResultRow r = it.next();
-                    String path = r.getPath(selectorName);
-                    if (includeRow(path)) {
+                    Tree tree = r.getTree(selectorName);
+                    if (tree != null && tree.exists()) {
                         try {
-                            current = getNode(getLocalPath(path));
+                            current = getNode(tree);
                             break;
                         } catch (RepositoryException e) {
-                            LOG.warn("Unable to fetch result node for path " + path, e);
+                            LOG.warn("Unable to fetch result node for path "
+                                     + tree.getPath(), e);
                         }
                     }
                 }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/RowImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/RowImpl.java?rev=1545283&r1=1545282&r2=1545283&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/RowImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/RowImpl.java Mon Nov 25 13:54:37 2013
@@ -26,6 +26,7 @@ import javax.jcr.query.Row;
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.ResultRow;
 import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
 import org.apache.jackrabbit.oak.spi.query.PropertyValues;
 
 import com.google.common.base.Joiner;
@@ -47,18 +48,18 @@ public class RowImpl implements Row {
 
     @Override
     public Node getNode() throws RepositoryException {
-        return result.getNode(getPath());
+        return getNode(null);
     }
 
     @Override
     public Node getNode(String selectorName) throws RepositoryException {
-        return result.getNode(getPath(selectorName));
+        return result.getNode(row.getTree(selectorName));
     }
 
     @Override
     public String getPath() throws RepositoryException {
         try {
-            return result.getLocalPath(row.getPath(pathSelector));
+            return row.getPath(pathSelector);
         } catch (IllegalArgumentException e) {
             throw new RepositoryException(e);
         }
@@ -67,7 +68,7 @@ public class RowImpl implements Row {
     @Override
     public String getPath(String selectorName) throws RepositoryException {
         try {
-            return result.getLocalPath(row.getPath(selectorName));
+            return row.getPath(selectorName);
         } catch (IllegalArgumentException e) {
             throw new RepositoryException(e);
         }