From e329115ab0077de96d3b5e0881de11f41bac91a5 Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Thu, 31 Oct 2013 09:43:28 -0400 Subject: [PATCH] OAK-1130: Performance issues with MutableTree Preload the Tree in a selector for each current row to avoid repeated path lookups --- .../jackrabbit/oak/query/ast/AstElement.java | 5 ----- .../oak/query/ast/FullTextSearchImpl.java | 21 +++++++++-------- .../oak/query/ast/PropertyValueImpl.java | 2 +- .../jackrabbit/oak/query/ast/SelectorImpl.java | 26 +++++++++++++--------- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java index 88e5c39..40786b4 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java @@ -19,7 +19,6 @@ package org.apache.jackrabbit.oak.query.ast; import org.apache.jackrabbit.oak.api.PropertyValue; -import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.query.QueryImpl; import org.apache.jackrabbit.oak.spi.query.PropertyValues; @@ -122,9 +121,5 @@ abstract class AstElement { return path; } - protected Tree getTree(String path) { - return query.getTree(path); - } - } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java index ea98866..bfa505d 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java @@ -20,6 +20,7 @@ package org.apache.jackrabbit.oak.query.ast; import static org.apache.jackrabbit.oak.api.Type.STRING; import static org.apache.jackrabbit.oak.api.Type.STRINGS; +import static org.apache.jackrabbit.oak.commons.PathUtils.elements; import java.text.ParseException; import java.util.Collections; @@ -182,21 +183,23 @@ public class FullTextSearchImpl extends ConstraintImpl { } appendString(buff, p); } else { - String path = selector.currentPath(); - if (!PathUtils.denotesRoot(path)) { - appendString(buff, - PropertyValues.newString(PathUtils.getName(path))); + Tree tree = selector.currentTree(); + if (tree == null) { + return false; } if (relativePath != null) { - String rp = normalizePath(relativePath); - path = PathUtils.concat(path, rp); + for (String element : elements(normalizePath(relativePath))) { + tree = tree.getChild(element); + } } - - Tree tree = getTree(path); - if (tree == null || !tree.exists()) { + if (!tree.exists()) { return false; } + if (!tree.isRoot()) { + appendString(buff, PropertyValues.newString(tree.getName())); + } + if (propertyName != null) { String pn = normalizePropertyName(propertyName); PropertyState p = tree.getProperty(pn); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java index 83cb9f0..f1eb907 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java @@ -112,7 +112,7 @@ public class PropertyValueImpl extends DynamicOperandImpl { PropertyValue p = selector.currentProperty(propertyName); return matchesPropertyType(p) ? p : null; } - Tree tree = getTree(selector.currentPath()); + Tree tree = selector.currentTree(); if (tree == null || !tree.exists()) { return null; } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java index ae1a8bb..da69048 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java @@ -35,6 +35,7 @@ import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_S import java.util.ArrayList; import java.util.Set; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import org.apache.jackrabbit.oak.api.PropertyState; @@ -91,8 +92,9 @@ public class SelectorImpl extends SourceImpl { */ private final Set mixinTypes; - private Cursor cursor; - private IndexRow currentRow; + private Cursor cursor = null; + private IndexRow currentRow = null; + private Tree tree = null; private int scanCount; /** @@ -269,6 +271,7 @@ public class SelectorImpl extends SourceImpl { while (cursor != null && cursor.hasNext()) { scanCount++; currentRow = cursor.next(); + tree = query.getTree(currentRow.getPath()); if (isParent) { // we must not check whether the _parent_ is readable // for joins of type @@ -289,7 +292,6 @@ public class SelectorImpl extends SourceImpl { // where [a].[jcr:path] = $path" // because not checking would reveal existence // of the child node - Tree tree = getTree(currentRow.getPath()); if (tree == null || !tree.exists()) { continue; } @@ -307,11 +309,11 @@ public class SelectorImpl extends SourceImpl { } cursor = null; currentRow = null; + tree = null; return false; } private boolean evaluateTypeMatch() { - Tree tree = getTree(currentRow.getPath()); if (tree == null || !tree.exists()) { return false; } @@ -340,16 +342,18 @@ public class SelectorImpl extends SourceImpl { * * @return the path */ + @CheckForNull public String currentPath() { - return cursor == null ? null : currentRow.getPath(); - } - - public Tree currentTree() { - String path = currentPath(); - if (path == null) { + if (tree != null) { + return tree.getPath(); + } else { return null; } - return getTree(path); + } + + @CheckForNull + public Tree currentTree() { + return tree; } /** -- 1.8.1.msysgit.1