nfsantos commented on code in PR #2113:
URL: https://github.com/apache/jackrabbit-oak/pull/2113#discussion_r1973404886
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java:
##########
@@ -588,37 +588,36 @@ public Matcher match(String name, NodeState nodeState) {
}
}
- public Collection<Matcher> nextSet() {
+ public List<Matcher> nextSet() {
checkArgument(status != Status.FAIL);
- if (status == Status.MATCH_FOUND){
+ if (status == Status.MATCH_FOUND) {
Aggregate nextAgg =
currentInclude.getAggregate(matchedNodeState);
- if (nextAgg != null){
+ if (nextAgg != null) {
int recursionLevel = aggregateStack.size() + 1;
- if (recursionLevel >=
rootState.rootAggregate.reAggregationLimit){
- return Collections.emptyList();
+ if (recursionLevel >=
rootState.rootAggregate.reAggregationLimit) {
+ return List.of();
}
List<Matcher> result = new
ArrayList<>(nextAgg.includes.size());
- for (Include i : nextAgg.includes){
- result.add(new Matcher(this, i, currentPath));
+ for (Include i : nextAgg.includes) {
+ result.add(new Matcher(this, i, currentPath));
}
return result;
}
- return Collections.emptyList();
+ return List.of();
Review Comment:
Several reasons:
- `List.of()` is a more modern API and seems to be the preferred choice for
new APIs.
- It reads better and is more maintainable when used with the other static
factory constructors in the List class:
```
List.of()
List.of("a")
List.of(1,2,3)
```
If we want to change between empty and non-empty, it is easier. And is more
consistent.
- Performance may also be better is using only List.of() for every list used
in a particular code path. This is because List.of() will return only 2
different implementations of the List class, which will mean that any call site
that uses these objects will be mono or bimorphic. The JIT compiler can
optimize these call sites to have similar performance as static calls. However,
if there are more than 2 types of lists, the call site becomes polimorphic and
performance is worse. Usually, this will have no measurable difference in
performance, but in performance critical loops like here, the impact can be
significant.
https://stackoverflow.com/questions/33317720/performance-of-collections-emptylist-and-empty-arraylist-with-jit-compiler
--
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: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
