HUSTERGS commented on code in PR #16201:
URL: https://github.com/apache/lucene/pull/16201#discussion_r3394023626
##########
lucene/core/src/java/org/apache/lucene/internal/hppc/IntObjectHashMap.java:
##########
@@ -562,7 +562,7 @@ public ValuesContainer values() {
}
/** A view over the set of values of this map. */
- public final class ValuesContainer implements Iterable<ObjectCursor<VType>> {
+ public class ValuesContainer implements Iterable<ObjectCursor<VType>> {
Review Comment:
@iprithv Thanks, I agree with your point. Extending `IntObjectHashMap` makes
the dense representation depend on implementation details of the parent class,
and it also forces changes like making `KeysContainer` / `ValuesContainer`
non-final. Composition is cleaner from that perspective. I've updated the code
according to your suggestion.
I am now not fully sure whether this additional maintenance cost is
justified by the expected heap saving, even though the optimization is useful
for our workload with many segments and many fields per segment.
What do you think? Is this becoming too complex for the expected benefit?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
