This is an automated email from the ASF dual-hosted git repository.
nfsantos pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 839c982efa OAK-11505 - Reduce object allocation in FullTextIndexEditor
(#2098)
839c982efa is described below
commit 839c982efaefcc21d631eb3f71398872e1a5a4f7
Author: Nuno Santos <[email protected]>
AuthorDate: Mon Feb 24 11:18:54 2025 +0100
OAK-11505 - Reduce object allocation in FullTextIndexEditor (#2098)
---
.../search/spi/editor/FulltextDocumentMaker.java | 1 -
.../search/spi/editor/FulltextIndexEditor.java | 135 ++++++++++++---------
2 files changed, 75 insertions(+), 61 deletions(-)
diff --git
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextDocumentMaker.java
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextDocumentMaker.java
index 72d63fa238..ab1df0dfc0 100644
---
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextDocumentMaker.java
+++
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextDocumentMaker.java
@@ -723,5 +723,4 @@ public abstract class FulltextDocumentMaker<D> implements
DocumentMaker<D> {
//cameCase file name to allow faster like search
indexNodeName(doc, value);
}
-
}
diff --git
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
index 9c70a511ea..cf51a33b0c 100644
---
a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
+++
b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java
@@ -22,7 +22,6 @@ import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.commons.collections.IterableUtils;
-import org.apache.jackrabbit.oak.commons.collections.SetUtils;
import org.apache.jackrabbit.oak.plugins.index.IndexEditor;
import org.apache.jackrabbit.oak.plugins.index.search.Aggregate;
import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition;
@@ -37,22 +36,20 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collections;
+import java.util.BitSet;
import java.util.List;
-import java.util.Set;
-
-import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
/**
* Generic implementation of an {@link IndexEditor} which supports index time
aggregation.
*/
public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateRoot {
- private static final Logger log =
- LoggerFactory.getLogger(FulltextIndexEditor.class);
+ private static final Logger log =
LoggerFactory.getLogger(FulltextIndexEditor.class);
public static final String TEXT_EXTRACTION_ERROR = "TextExtractionError";
+ private static final List<Aggregate.Matcher> EMPTY_AGGREGATE_MATCHER_LIST
= List.of();
+
private final FulltextIndexEditorContext<D> context;
/* Name of this node, or {@code null} for the root node. */
@@ -75,10 +72,12 @@ public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateR
private IndexDefinition.IndexingRule indexingRule;
- private List<Aggregate.Matcher> currentMatchers = Collections.emptyList();
+ private List<Aggregate.Matcher> currentMatchers = List.of();
private final MatcherState matcherState;
+ private final PathFilter pathFilter;
+
private final PathFilter.Result pathFilterResult;
public FulltextIndexEditor(FulltextIndexEditorContext<D> context) {
@@ -88,11 +87,13 @@ public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateR
this.context = context;
this.isDeleted = false;
this.matcherState = MatcherState.NONE;
- this.pathFilterResult =
context.getDefinition().getPathFilter().filter(PathUtils.ROOT_PATH);
+ this.pathFilter = context.getDefinition().getPathFilter();
+ this.pathFilterResult = this.pathFilter.filter(PathUtils.ROOT_PATH);
}
public FulltextIndexEditor(FulltextIndexEditor<D> parent, String name,
MatcherState matcherState,
+ PathFilter pathFilter,
PathFilter.Result pathFilterResult,
boolean isDeleted) {
this.parent = parent;
@@ -101,12 +102,13 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
this.context = parent.context;
this.isDeleted = isDeleted;
this.matcherState = matcherState;
+ this.pathFilter = pathFilter;
this.pathFilterResult = pathFilterResult;
}
public String getPath() {
if (path == null) { // => parent != null
- path = concat(parent.getPath(), name);
+ path = PathUtils.concat(parent.getPath(), name);
}
return path;
}
@@ -118,8 +120,7 @@ public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateR
}
//Only check for indexing if the result is include.
- //In case like TRAVERSE nothing needs to be indexed for those
- //path
+ //In case like TRAVERSE nothing needs to be indexed for those paths
if (pathFilterResult == PathFilter.Result.INCLUDE) {
//For traversal in deleted sub tree before state has to be used
NodeState current = after.exists() ? after : before;
@@ -144,7 +145,9 @@ public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateR
}
}
- for (Aggregate.Matcher m : matcherState.affectedMatchers) {
+ BitSet bitSet = matcherState.affectedMatchers;
+ for (int i = bitSet.nextSetBit(0); i != -1; i = bitSet.nextSetBit(i +
1)) {
+ Aggregate.Matcher m = matcherState.matched.get(i);
m.markRootDirty();
}
@@ -178,7 +181,9 @@ public class FulltextIndexEditor<D> implements IndexEditor,
Aggregate.AggregateR
@Override
public void propertyChanged(PropertyState before, PropertyState after) {
markPropertyChanged(before.getName());
- propertiesModified.add(before);
+ if (isIndexable()) {
+ propertiesModified.add(before);
+ }
checkAggregates(before.getName());
propertyUpdated(before, after);
}
@@ -186,45 +191,48 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
@Override
public void propertyDeleted(PropertyState before) {
markPropertyChanged(before.getName());
- propertiesModified.add(before);
+ if (isIndexable()) {
+ propertiesModified.add(before);
+ }
checkAggregates(before.getName());
propertyUpdated(before, null);
}
@Override
public Editor childNodeAdded(String name, NodeState after) {
- PathFilter.Result filterResult = getPathFilterResult(name);
+ PathFilter.Result filterResult =
pathFilter.filter(PathUtils.concat(getPath(), name));
if (filterResult != PathFilter.Result.EXCLUDE) {
- return new FulltextIndexEditor<>(this, name, getMatcherState(name,
after), filterResult, false);
+ return new FulltextIndexEditor<>(this, name, getMatcherState(name,
after), pathFilter, filterResult, false);
+ } else {
+ return null;
}
- return null;
}
@Override
- public Editor childNodeChanged(
- String name, NodeState before, NodeState after) {
- PathFilter.Result filterResult = getPathFilterResult(name);
+ public Editor childNodeChanged(String name, NodeState before, NodeState
after) {
+ PathFilter.Result filterResult =
pathFilter.filter(PathUtils.concat(getPath(), name));
if (filterResult != PathFilter.Result.EXCLUDE) {
- return new FulltextIndexEditor<>(this, name, getMatcherState(name,
after), filterResult, false);
+ return new FulltextIndexEditor<>(this, name, getMatcherState(name,
after), pathFilter, filterResult, false);
+ } else {
+ return null;
}
- return null;
}
@Override
public Editor childNodeDeleted(String name, NodeState before)
throws CommitFailedException {
- PathFilter.Result filterResult = getPathFilterResult(name);
+ String childPath = PathUtils.concat(getPath(), name);
+ PathFilter.Result filterResult = pathFilter.filter(childPath);
if (filterResult == PathFilter.Result.EXCLUDE) {
return null;
}
if (!isDeleted) {
// tree deletion is handled on the parent node
- String path = concat(getPath(), name);
try {
FulltextIndexWriter<D> writer = context.getWriter();
// Remove all index entries in the removed subtree
- writer.deleteDocuments(path);
+ writer.deleteDocuments(childPath);
this.context.indexUpdate();
} catch (IOException e) {
CommitFailedException ce = new
CommitFailedException("Fulltext", 5, "Failed to remove the index entries of"
@@ -235,10 +243,11 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
}
MatcherState ms = getMatcherState(name, before);
- if (!ms.isEmpty()) {
- return new FulltextIndexEditor<>(this, name, ms, filterResult,
true);
+ if (ms.isEmpty()) {
+ return null; // no need to recurse down the removed subtree
+ } else {
+ return new FulltextIndexEditor<>(this, name, ms, pathFilter,
filterResult, true);
}
- return null; // no need to recurse down the removed subtree
}
public FulltextIndexEditorContext<D> getContext() {
@@ -270,10 +279,11 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
}
private D makeDocument(String path, NodeState state, boolean isUpdate)
throws IOException {
- if (!isIndexable()) {
+ if (isIndexable()) {
+ return context.newDocumentMaker(indexingRule,
path).makeDocument(state, isUpdate, propertiesModified);
+ } else {
return null;
}
- return context.newDocumentMaker(indexingRule,
path).makeDocument(state, isUpdate, propertiesModified);
}
@@ -285,23 +295,34 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
}
private MatcherState getMatcherState(String name, NodeState after) {
- List<Aggregate.Matcher> matched = new ArrayList<>();
- List<Aggregate.Matcher> inherited = new ArrayList<>();
+ // Short circuit if there are no matchers to avoid creating the
iterator over these two lists
+ if (matcherState.inherited.isEmpty() && currentMatchers.isEmpty()) {
+ return MatcherState.NONE;
+ }
+ List<Aggregate.Matcher> matched = EMPTY_AGGREGATE_MATCHER_LIST;
+ List<Aggregate.Matcher> inherited = EMPTY_AGGREGATE_MATCHER_LIST;
for (Aggregate.Matcher m :
IterableUtils.chainedIterable(matcherState.inherited, currentMatchers)) {
Aggregate.Matcher result = m.match(name, after);
if (result.getStatus() == Aggregate.Matcher.Status.MATCH_FOUND) {
+ if (matched == EMPTY_AGGREGATE_MATCHER_LIST) {
+ matched = new ArrayList<>(4);
+ }
matched.add(result);
}
if (result.getStatus() != Aggregate.Matcher.Status.FAIL) {
+ if (inherited == EMPTY_AGGREGATE_MATCHER_LIST) {
+ inherited = new ArrayList<>(4);
+ }
inherited.addAll(result.nextSet());
}
}
- if (!matched.isEmpty() || !inherited.isEmpty()) {
+ if (matched.isEmpty() && inherited.isEmpty()) {
+ return MatcherState.NONE;
+ } else {
return new MatcherState(matched, inherited);
}
- return MatcherState.NONE;
}
@@ -311,33 +332,30 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
* @param name modified property name
*/
private void checkAggregates(String name) {
- for (Aggregate.Matcher m : matcherState.matched) {
- if (!matcherState.affectedMatchers.contains(m)
- && m.aggregatesProperty(name)) {
- matcherState.affectedMatchers.add(m);
+ // Performance critical code, iterate using an index to avoid
allocating an iterator
+ for (int i = 0; i < matcherState.matched.size(); i++) {
+ if (!matcherState.affectedMatchers.get(i)) {
+ Aggregate.Matcher m = matcherState.matched.get(i);
+ if (m.aggregatesProperty(name)) {
+ matcherState.affectedMatchers.set(i);
+ }
}
}
}
- static class MatcherState {
+ public static class MatcherState {
+ private final static BitSet EMPTY_BITSET = new BitSet(0);
final static MatcherState NONE = new MatcherState(List.of(),
List.of());
final List<Aggregate.Matcher> matched;
final List<Aggregate.Matcher> inherited;
- final Set<Aggregate.Matcher> affectedMatchers;
+ final BitSet affectedMatchers;
- public MatcherState(List<Aggregate.Matcher> matched,
- List<Aggregate.Matcher> inherited) {
+ public MatcherState(List<Aggregate.Matcher> matched,
List<Aggregate.Matcher> inherited) {
this.matched = matched;
this.inherited = inherited;
-
- //Affected matches would only be used when there are
- //some matched matchers
- if (matched.isEmpty()) {
- affectedMatchers = Collections.emptySet();
- } else {
- affectedMatchers = SetUtils.newIdentityHashSet();
- }
+ // Affected matches would only be used when there are some matched
matchers
+ this.affectedMatchers = matched.isEmpty() ? EMPTY_BITSET : new
BitSet(matched.size());
}
public boolean isEmpty() {
@@ -370,13 +388,14 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
}
}
- for (Aggregate.Matcher m : matcherState.matched) {
+ // Performance critical code, iterate using an index to avoid
allocating an iterator
+ for (int i = 0; i < matcherState.matched.size(); i++) {
+ Aggregate.Matcher m = matcherState.matched.get(i);
if (m.aggregatesProperty(propertyName)) {
- Aggregate.Include i = m.getCurrentInclude();
- if (i instanceof Aggregate.PropertyInclude) {
- PropertyDefinition pd = ((Aggregate.PropertyInclude)
i).getPropertyDefinition();
+ Aggregate.Include aggregateInclude = m.getCurrentInclude();
+ if (aggregateInclude instanceof Aggregate.PropertyInclude) {
+ PropertyDefinition pd = ((Aggregate.PropertyInclude)
aggregateInclude).getPropertyDefinition();
String propertyRelativePath =
PathUtils.concat(m.getMatchedPath(), propertyName);
-
callback.propertyUpdated(m.getRootPath(),
propertyRelativePath, pd, before, after);
}
}
@@ -391,10 +410,6 @@ public class FulltextIndexEditor<D> implements
IndexEditor, Aggregate.AggregateR
return indexingRule != null;
}
- private PathFilter.Result getPathFilterResult(String childNodeName) {
- return
context.getDefinition().getPathFilter().filter(concat(getPath(),
childNodeName));
- }
-
private String getIndexName() {
return context.getDefinition().getIndexName();
}
