nfsantos commented on code in PR #2108:
URL: https://github.com/apache/jackrabbit-oak/pull/2108#discussion_r1975189401
##########
oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java:
##########
@@ -1395,27 +1400,27 @@ private Map<String, PropertyDefinition>
collectPropConfigs(NodeState config,
}
}
ensureNodeTypeIndexingIsConsistent(propDefns, syncProps);
- return Map.copyOf(propDefns);
Review Comment:
I don't see a reason why we must make a copy. Once this method finishes,
there is no other reference to the map other than the one returned by the
method, so there is no risk of the map being modified elsewhere. And the
IndexDefinition class does not modify the map anywhere, just reads it. So I
don't think it is necessary to make a copy or to make the map immutable. Maybe
it was done just as a best-practice to ensure immutability. Or as an attempt at
performance optimization, as an immutable map created by `Map.copyOf()` may be
faster than a HashMap due to a more efficient internal representation. I am not
sure, but in this case, avoiding the creation of lower case strings is a big
gain, easily offsets any additional overhead of searching for a key on a tree
as compared to search on a hashmap. Searching on a HashMap also requires
computing the hash of the String, which can be expensive.
--
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
