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

Reply via email to