nfsantos commented on code in PR #2098:
URL: https://github.com/apache/jackrabbit-oak/pull/2098#discussion_r1965493034
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/editor/FulltextIndexEditor.java:
##########
@@ -285,61 +296,73 @@ public void markDirty() {
}
private MatcherState getMatcherState(String name, NodeState after) {
- List<Aggregate.Matcher> matched = new ArrayList<>();
- List<Aggregate.Matcher> inherited = new ArrayList<>();
+ 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;
}
+
/*
* Determines which all matchers are affected by this property change
*
* @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);
+ // Avoid allocating the iterator over matcherState.matched in the
common case of the list being empty
+ if (!matcherState.matched.isEmpty()) {
Review Comment:
I found this when looking at a JFR profile of an indexing job using the
OutOfBandIndexer. The allocation of the iterators over `matcherState.matched`
was the 3rd or 4th largest source of object allocations. I was also surprised,
I thought the JIT would be able to optimize away this allocation. It made me
wonder if in performance critical regions, it may be better to iterate with an
old-fashioned for loop with an explicit index, `for (int i=0; i<c.length...)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
