jtibshirani commented on a change in pull request #1948: URL: https://github.com/apache/lucene-solr/pull/1948#discussion_r505869567
########## File path: lucene/core/src/java/org/apache/lucene/index/OrdinalMap.java ########## @@ -359,10 +375,9 @@ public long ramBytesUsed() { @Override public Collection<Accountable> getChildResources() { List<Accountable> resources = new ArrayList<>(); - resources.add(Accountables.namedAccountable("global ord deltas", globalOrdDeltas)); - resources.add(Accountables.namedAccountable("first segments", firstSegments)); resources.add(Accountables.namedAccountable("segment map", segmentMap)); - // TODO: would be nice to return actual child segment deltas too, but the optimizations are confusing + // TODO: would be nice to return the ordinal and segment maps too, but it's not straightforward + // because of optimizations. Review comment: I think we'd need a cast here, since `LongValues` doesn't implement `Accountable`. Alternatively, we could consider a bigger change to have `LongValues` implement `Accountable`. Update: I just saw LUCENE-9387, so it probably doesn't make sense to increase usage of `Accountable`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org