Remove Accountable interface in KnnVectorsReader (apache#13255)

shatejas · Apr 8, 2024 · df0384c · df0384c
1 parent 4c843fc
commit df0384c
Show file tree

Hide file tree

Showing 15 changed files with 10 additions and 114 deletions.
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
@@ -96,6 +96,9 @@ API Changes
 
 * GITHUB#13261: Convert `BooleanClause` class to record class. (Pulkit Gupta)
 
+* GITHUB#13241: Remove Accountable interface on KnnVectorsReader. (Pulkit Gupta)
+
+
 New Features
 ---------------------
 

diff --git a/lucene/MIGRATE.md b/lucene/MIGRATE.md
@@ -225,6 +225,12 @@ public class CustomCollectorManager implements CollectorManager<CustomCollector,
 
 List<Object> results = searcher.search(query, new CustomCollectorManager());
 ```
+
+### Accountable interface removed from KnnVectorsReader (GITHUB#13255)
+
+`KnnVectorsReader` objects use small heap memory, so it's not worth maintaining heap usage for them hence removed
+`Accountable` interface from `KnnVectorsReader`.
+
 ## Migration from Lucene 9.0 to Lucene 9.1
 
 ### Test framework package migration and module (LUCENE-10301)

diff --git a/...codecs/src/java/org/apache/lucene/backward_codecs/lucene90/Lucene90HnswVectorsReader.java b/...codecs/src/java/org/apache/lucene/backward_codecs/lucene90/Lucene90HnswVectorsReader.java
@@ -40,7 +40,6 @@
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.RamUsageEstimator;
 import org.apache.lucene.util.hnsw.HnswGraph;
 import org.apache.lucene.util.hnsw.NeighborQueue;
 import org.apache.lucene.util.hnsw.RandomAccessVectorValues;
@@ -214,18 +213,6 @@ private FieldEntry readField(IndexInput input, FieldInfo info) throws IOExceptio
     return new FieldEntry(input, info.getVectorSimilarityFunction());
   }
 
-  @Override
-  public long ramBytesUsed() {
-    long totalBytes = RamUsageEstimator.shallowSizeOfInstance(Lucene90HnswVectorsReader.class);
-    totalBytes +=
-        RamUsageEstimator.sizeOfMap(
-            fields, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-    for (FieldEntry entry : fields.values()) {
-      totalBytes += RamUsageEstimator.sizeOf(entry.ordToDoc);
-    }
-    return totalBytes;
-  }
-
   @Override
   public void checkIntegrity() throws IOException {
     CodecUtil.checksumEntireFile(vectorData);

diff --git a/...codecs/src/java/org/apache/lucene/backward_codecs/lucene91/Lucene91HnswVectorsReader.java b/...codecs/src/java/org/apache/lucene/backward_codecs/lucene91/Lucene91HnswVectorsReader.java
@@ -40,7 +40,6 @@
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.RamUsageEstimator;
 import org.apache.lucene.util.hnsw.HnswGraph;
 import org.apache.lucene.util.hnsw.HnswGraphSearcher;
 import org.apache.lucene.util.hnsw.OrdinalTranslatedKnnCollector;
@@ -206,18 +205,6 @@ private FieldEntry readField(IndexInput input, FieldInfo info) throws IOExceptio
     return new FieldEntry(input, info.getVectorSimilarityFunction());
   }
 
-  @Override
-  public long ramBytesUsed() {
-    long totalBytes = RamUsageEstimator.shallowSizeOfInstance(Lucene91HnswVectorsFormat.class);
-    totalBytes +=
-        RamUsageEstimator.sizeOfMap(
-            fields, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-    for (FieldEntry entry : fields.values()) {
-      totalBytes += RamUsageEstimator.sizeOf(entry.ordToDoc);
-    }
-    return totalBytes;
-  }
-
   @Override
   public void checkIntegrity() throws IOException {
     CodecUtil.checksumEntireFile(vectorData);

diff --git a/...codecs/src/java/org/apache/lucene/backward_codecs/lucene92/Lucene92HnswVectorsReader.java b/...codecs/src/java/org/apache/lucene/backward_codecs/lucene92/Lucene92HnswVectorsReader.java
@@ -39,7 +39,6 @@
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.RamUsageEstimator;
 import org.apache.lucene.util.hnsw.HnswGraph;
 import org.apache.lucene.util.hnsw.HnswGraphSearcher;
 import org.apache.lucene.util.hnsw.OrdinalTranslatedKnnCollector;
@@ -205,15 +204,6 @@ private FieldEntry readField(IndexInput input, FieldInfo info) throws IOExceptio
     return new FieldEntry(input, info.getVectorSimilarityFunction());
   }
 
-  @Override
-  public long ramBytesUsed() {
-    long totalBytes = RamUsageEstimator.shallowSizeOfInstance(Lucene92HnswVectorsFormat.class);
-    totalBytes +=
-        RamUsageEstimator.sizeOfMap(
-            fields, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-    return totalBytes;
-  }
-
   @Override
   public void checkIntegrity() throws IOException {
     CodecUtil.checksumEntireFile(vectorData);

diff --git a/...codecs/src/java/org/apache/lucene/backward_codecs/lucene94/Lucene94HnswVectorsReader.java b/...codecs/src/java/org/apache/lucene/backward_codecs/lucene94/Lucene94HnswVectorsReader.java
@@ -40,7 +40,6 @@
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.RamUsageEstimator;
 import org.apache.lucene.util.hnsw.HnswGraph;
 import org.apache.lucene.util.hnsw.HnswGraphSearcher;
 import org.apache.lucene.util.hnsw.OrdinalTranslatedKnnCollector;
@@ -223,15 +222,6 @@ private FieldEntry readField(IndexInput input, FieldInfo info) throws IOExceptio
     return new FieldEntry(input, vectorEncoding, info.getVectorSimilarityFunction());
   }
 
-  @Override
-  public long ramBytesUsed() {
-    long totalBytes = RamUsageEstimator.shallowSizeOfInstance(Lucene94HnswVectorsFormat.class);
-    totalBytes +=
-        RamUsageEstimator.sizeOfMap(
-            fields, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-    return totalBytes;
-  }
-
   @Override
   public void checkIntegrity() throws IOException {
     CodecUtil.checksumEntireFile(vectorData);

diff --git a/...codecs/src/java/org/apache/lucene/backward_codecs/lucene95/Lucene95HnswVectorsReader.java b/...codecs/src/java/org/apache/lucene/backward_codecs/lucene95/Lucene95HnswVectorsReader.java
@@ -61,9 +61,6 @@
  */
 public final class Lucene95HnswVectorsReader extends KnnVectorsReader implements HnswGraphProvider {
 
-  private static final long SHALLOW_SIZE =
-      RamUsageEstimator.shallowSizeOfInstance(Lucene95HnswVectorsFormat.class);
-
   private final FieldInfos fieldInfos;
   private final Map<String, FieldEntry> fields = new HashMap<>();
   private final IndexInput vectorData;
@@ -235,13 +232,6 @@ private FieldEntry readField(IndexInput input, FieldInfo info) throws IOExceptio
     return new FieldEntry(input, vectorEncoding, info.getVectorSimilarityFunction());
   }
 
-  @Override
-  public long ramBytesUsed() {
-    return Lucene95HnswVectorsReader.SHALLOW_SIZE
-        + RamUsageEstimator.sizeOfMap(
-            fields, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-  }
-
   @Override
   public void checkIntegrity() throws IOException {
     CodecUtil.checksumEntireFile(vectorData);

diff --git a/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextKnnVectorsReader.java b/lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextKnnVectorsReader.java
@@ -46,7 +46,6 @@
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 import org.apache.lucene.util.IOUtils;
-import org.apache.lucene.util.RamUsageEstimator;
 import org.apache.lucene.util.StringHelper;
 import org.apache.lucene.util.hnsw.RandomAccessVectorValues;
 
@@ -58,9 +57,6 @@
  */
 public class SimpleTextKnnVectorsReader extends KnnVectorsReader {
   // shallowSizeOfInstance for fieldEntries map is included in ramBytesUsed() calculation
-  private static final long BASE_RAM_BYTES_USED =
-      RamUsageEstimator.shallowSizeOfInstance(SimpleTextKnnVectorsReader.class)
-          + RamUsageEstimator.shallowSizeOfInstance(BytesRef.class);
 
   private static final BytesRef EMPTY = new BytesRef("");
 
@@ -274,20 +270,6 @@ public void checkIntegrity() throws IOException {
     }
   }
 
-  @Override
-  public long ramBytesUsed() {
-    // mirror implementation of Lucene90VectorReader#ramBytesUsed
-    long totalBytes = BASE_RAM_BYTES_USED;
-    totalBytes += RamUsageEstimator.sizeOf(scratch.bytes());
-    totalBytes +=
-        RamUsageEstimator.sizeOfMap(
-            fieldEntries, RamUsageEstimator.shallowSizeOfInstance(FieldEntry.class));
-    for (FieldEntry entry : fieldEntries.values()) {
-      totalBytes += RamUsageEstimator.sizeOf(entry.ordToDoc);
-    }
-    return totalBytes;
-  }
-
   @Override
   public void close() throws IOException {
     dataIn.close();

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsFormat.java b/lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsFormat.java
@@ -131,11 +131,6 @@ public void search(
 
             @Override
             public void close() {}
-
-            @Override
-            public long ramBytesUsed() {
-              return 0;
-            }
           };
         }
 

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsReader.java
@@ -26,11 +26,10 @@
 import org.apache.lucene.search.ScoreDoc;
 import org.apache.lucene.search.TopDocs;
 import org.apache.lucene.search.TotalHits;
-import org.apache.lucene.util.Accountable;
 import org.apache.lucene.util.Bits;
 
 /** Reads vectors from an index. */
-public abstract class KnnVectorsReader implements Closeable, Accountable {
+public abstract class KnnVectorsReader implements Closeable {
 
   /** Sole constructor */
   protected KnnVectorsReader() {}

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldKnnVectorsFormat.java b/lucene/core/src/java/org/apache/lucene/codecs/perfield/PerFieldKnnVectorsFormat.java
@@ -286,15 +286,6 @@ public void search(String field, byte[] target, KnnCollector knnCollector, Bits
     public void close() throws IOException {
       IOUtils.close(fields.values());
     }
-
-    @Override
-    public long ramBytesUsed() {
-      long total = 0;
-      for (KnnVectorsReader reader : fields.values()) {
-        total += reader.ramBytesUsed();
-      }
-      return total;
-    }
   }
 
   static String getSuffix(String formatName, String suffix) {

diff --git a/lucene/core/src/java/org/apache/lucene/index/SlowCodecReaderWrapper.java b/lucene/core/src/java/org/apache/lucene/index/SlowCodecReaderWrapper.java
@@ -191,11 +191,6 @@ public void checkIntegrity() {
 
       @Override
       public void close() {}
-
-      @Override
-      public long ramBytesUsed() {
-        return 0L;
-      }
     };
   }
 

diff --git a/lucene/core/src/java/org/apache/lucene/index/SlowCompositeCodecReaderWrapper.java b/lucene/core/src/java/org/apache/lucene/index/SlowCompositeCodecReaderWrapper.java
@@ -823,15 +823,6 @@ public void close() throws IOException {
       IOUtils.close(readers);
     }
 
-    @Override
-    public long ramBytesUsed() {
-      long ramBytesUsed = 0;
-      for (KnnVectorsReader reader : readers) {
-        ramBytesUsed += reader.ramBytesUsed();
-      }
-      return ramBytesUsed;
-    }
-
     @Override
     public void checkIntegrity() throws IOException {
       for (KnnVectorsReader reader : readers) {

diff --git a/lucene/core/src/java/org/apache/lucene/index/SortingCodecReader.java b/lucene/core/src/java/org/apache/lucene/index/SortingCodecReader.java
@@ -520,11 +520,6 @@ public void search(String field, byte[] target, KnnCollector knnCollector, Bits
       public void close() throws IOException {
         delegate.close();
       }
-
-      @Override
-      public long ramBytesUsed() {
-        return delegate.ramBytesUsed();
-      }
     };
   }
 

diff --git a/...ramework/src/java/org/apache/lucene/tests/codecs/asserting/AssertingKnnVectorsFormat.java b/...ramework/src/java/org/apache/lucene/tests/codecs/asserting/AssertingKnnVectorsFormat.java
@@ -171,11 +171,6 @@ public void close() throws IOException {
       delegate.close();
     }
 
-    @Override
-    public long ramBytesUsed() {
-      return delegate.ramBytesUsed();
-    }
-
     @Override
     public HnswGraph getGraph(String field) throws IOException {
       return ((HnswGraphProvider) delegate).getGraph(field);
-Original file line number
+Diff line change
@@ Expand Up / @@ -96,6 +96,9 @@ API Changes @@
     * GITHUB#13261: Convert `BooleanClause` class to record class. (Pulkit Gupta)
+    * GITHUB#13241: Remove Accountable interface on KnnVectorsReader. (Pulkit Gupta)
     New Features
     ---------------------
@@ Expand Down @@