Remove BytesReference#copy

iverase · Oct 15, 2023 · 676cf76 · 676cf76
1 parent 4ee5fb0
commit 676cf76
Show file tree

Hide file tree

Showing 11 changed files with 7 additions and 134 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java b/server/src/main/java/org/elasticsearch/common/bytes/BytesArray.java
@@ -92,14 +92,6 @@ public BytesReference slice(int from, int length) {
         return new BytesArray(bytes, offset + from, length);
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        Objects.checkFromIndexSize(from, length, this.length);
-        final byte[] copy = new byte[length];
-        System.arraycopy(bytes, offset + from, copy, 0, length);
-        return new BytesArray(copy);
-    }
-
     @Override
     public boolean hasArray() {
         return true;

diff --git a/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java
@@ -153,17 +153,10 @@ static BytesReference fromByteArray(ByteArray byteArray, int length) {
     int length();
 
     /**
-     * Slice the bytes from the {@code from} index up to {@code length}. The slice contains
-     * a direct reference to the internal pages.
+     * Slice the bytes from the {@code from} index up to {@code length}.
      */
     BytesReference slice(int from, int length);
 
-    /**
-     * Make a copy the bytes from the {@code from} index up to {@code length}. The copy does not
-     * contain a direct reference to the internal pages.
-     */
-    BytesReference copy(int from, int length);
-
     /**
      * The amount of memory used by this BytesReference
      */

diff --git a/server/src/main/java/org/elasticsearch/common/bytes/CompositeBytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/CompositeBytesReference.java
@@ -164,26 +164,6 @@ public BytesReference slice(int from, int length) {
         return CompositeBytesReference.ofMultiple(inSlice);
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        Objects.checkFromIndexSize(from, length, this.length);
-        final int to = from + length;
-        final int limit = getOffsetIndex(to - 1);
-        final int start = getOffsetIndex(from);
-        final int numCopies = 1 + (limit - start);
-        final int inCopyOffset = from - offsets[start];
-        if (numCopies == 1) {
-            return references[start].copy(inCopyOffset, length);
-        }
-        final BytesReference[] inCopy = new BytesReference[numCopies];
-        inCopy[0] = references[start].copy(inCopyOffset, references[start].length() - inCopyOffset);
-        for (int i = 1, j = start + 1; i < inCopy.length - 1; i++, j++) {
-            inCopy[i] = references[j].copy(0, references[j].length());
-        }
-        inCopy[inCopy.length - 1] = references[limit].copy(0, to - offsets[limit]);
-        return CompositeBytesReference.ofMultiple(inCopy);
-    }
-
     private int getOffsetIndex(int offset) {
         final int i = Arrays.binarySearch(offsets, offset);
         return i < 0 ? (-(i + 1)) - 1 : i;

diff --git a/server/src/main/java/org/elasticsearch/common/bytes/PagedBytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/PagedBytesReference.java
@@ -10,7 +10,6 @@
 
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefIterator;
-import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.ByteArray;
 import org.elasticsearch.common.util.PageCacheRecycler;
 
@@ -49,25 +48,6 @@ public BytesReference slice(int from, int length) {
         return new PagedBytesReference(byteArray, offset + from, length);
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        Objects.checkFromIndexSize(from, length, this.length);
-        final ByteArray byteArray = BigArrays.NON_RECYCLING_INSTANCE.newByteArray(length);
-        final int offset = this.offset + from;
-        final int initialFragmentSize = offset != 0 ? PAGE_SIZE - (offset % PAGE_SIZE) : PAGE_SIZE;
-        final BytesRef slice = new BytesRef();
-        int nextFragmentSize = Math.min(length, initialFragmentSize);
-        int position = 0;
-        while (nextFragmentSize != 0) {
-            final boolean materialized = this.byteArray.get(offset + position, nextFragmentSize, slice);
-            assert materialized == false : "iteration should be page aligned but array got materialized";
-            byteArray.set(position, slice.bytes, slice.offset, slice.length);
-            position += nextFragmentSize;
-            nextFragmentSize = Math.min(length - position, PAGE_SIZE);
-        }
-        return BytesReference.fromByteArray(byteArray, length);
-    }
-
     @Override
     public BytesRef toBytesRef() {
         BytesRef bref = new BytesRef();

diff --git a/server/src/main/java/org/elasticsearch/common/bytes/ReleasableBytesReference.java b/server/src/main/java/org/elasticsearch/common/bytes/ReleasableBytesReference.java
@@ -139,12 +139,6 @@ public BytesReference slice(int from, int length) {
         return delegate.slice(from, length);
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        assert hasReferences();
-        return delegate.copy(from, length);
-    }
-
     @Override
     public long ramBytesUsed() {
         return delegate.ramBytesUsed();

diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java
@@ -164,12 +164,12 @@ public BytesReference readBytesReference(int length) throws IOException {
         if (length == 0) {
             return BytesArray.EMPTY;
         } else if (length < ByteSizeValue.ofMb(1).getBytes()) {
-            // if the bytes reference is small enough we can just copy the bytes in a single array
+            // if the BytesReference is less that 1MB we copy the bytes into a single byte array
             byte[] bytes = new byte[length];
             readBytes(bytes, 0, length);
             return new BytesArray(bytes, 0, length);
         } else {
-            // paginate the bytes reference to avoid allocating a single byte array that is too large
+            // paginate the BytesReference to avoid humongous allocations
             final BytesReference br = BytesReference.fromByteArray(BigArrays.NON_RECYCLING_INSTANCE.newByteArray(length), length);
             final BytesRefIterator iterator = br.iterator();
             BytesRef bytesRef;

diff --git a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReference.java
@@ -46,11 +46,6 @@ public BytesReference slice(int from, int length) {
         return new ZeroBytesReference(length);
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        return slice(from, length);
-    }
-
     @Override
     public long ramBytesUsed() {
         return 0;

diff --git a/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java b/server/src/test/java/org/elasticsearch/common/bytes/ZeroBytesReferenceTests.java
@@ -8,6 +8,8 @@
 
 package org.elasticsearch.common.bytes;
 
+import java.io.IOException;
+
 import static org.hamcrest.Matchers.containsString;
 
 public class ZeroBytesReferenceTests extends AbstractBytesReferenceTestCase {
@@ -37,20 +39,9 @@ public void testSliceToBytesRef() {
         // ZeroBytesReference shifts offsets
     }
 
-    @Override
-    public void testSlice() {
-        AssertionError error = expectThrows(AssertionError.class, super::testSlice);
-        assertThat(error.getMessage(), containsString("Internal pages from ZeroBytesReference must be zero"));
-    }
-
-    @Override
-    public void testCopy() {
-        AssertionError error = expectThrows(AssertionError.class, super::testCopy);
+    public void testWriteWithIterator() throws IOException {
+        AssertionError error = expectThrows(AssertionError.class, () -> super.testWriteWithIterator());
         assertThat(error.getMessage(), containsString("Internal pages from ZeroBytesReference must be zero"));
     }
 
-    public void testWriteWithIterator() {
-        AssertionError error = expectThrows(AssertionError.class, super::testWriteWithIterator);
-        assertThat(error.getMessage(), containsString("Internal pages from ZeroBytesReference must be zero"));
-    }
 }
diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesRequestCacheTests.java
@@ -594,11 +594,6 @@ public BytesReference slice(int from, int length) {
             return null;
         }
 
-        @Override
-        public BytesReference copy(int from, int length) {
-            return null;
-        }
-
         @Override
         public BytesRef toBytesRef() {
             return null;

diff --git a/...ramework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java b/...ramework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java
@@ -81,47 +81,6 @@ public void testSlice() throws IOException {
                 // the offset can be anywhere
                 assertEquals(sliceLength, singlePageOrNull.length);
             }
-            // make sure the slice is affected by changes to the original
-            final BytesRefIterator iterator = pbr.iterator();
-            BytesRef bytesRef;
-            while ((bytesRef = iterator.next()) != null) {
-                for (int i = 0; i < bytesRef.length; i++) {
-                    bytesRef.bytes[bytesRef.offset + i]++;
-                }
-            }
-            for (int i = 0; i < sliceLength; i++) {
-                assertEquals(pbr.get(i + sliceOffset), slice.get(i));
-            }
-        }
-    }
-
-    public void testCopy() throws IOException {
-        for (int length : new int[] { 0, 1, randomIntBetween(2, PAGE_SIZE), randomIntBetween(PAGE_SIZE + 1, 3 * PAGE_SIZE) }) {
-            BytesReference pbr = newBytesReference(length);
-            int sliceOffset = randomIntBetween(0, length / 2);
-            int copyLength = Math.max(0, length - sliceOffset - 1);
-            BytesReference slice = pbr.copy(sliceOffset, copyLength);
-            assertEquals(copyLength, slice.length());
-            for (int i = 0; i < copyLength; i++) {
-                assertEquals(pbr.get(i + sliceOffset), slice.get(i));
-            }
-            BytesRef singlePageOrNull = getSinglePageOrNull(slice);
-            if (singlePageOrNull != null) {
-                // we can't assert the offset since if the length is smaller than the refercence
-                // the offset can be anywhere
-                assertEquals(copyLength, singlePageOrNull.length);
-            }
-            // make sure copy is not affected by changes to the original
-            final BytesRefIterator iterator = pbr.iterator();
-            BytesRef bytesRef;
-            while ((bytesRef = iterator.next()) != null) {
-                for (int i = 0; i < bytesRef.length; i++) {
-                    bytesRef.bytes[bytesRef.offset + i]++;
-                }
-            }
-            for (int i = 0; i < copyLength; i++) {
-                assertEquals(pbr.get(i + sliceOffset), (byte) (slice.get(i) + 1));
-            }
         }
     }
 

diff --git a/...ava/org/elasticsearch/repositories/blobstore/testkit/RandomBlobContentBytesReference.java b/...ava/org/elasticsearch/repositories/blobstore/testkit/RandomBlobContentBytesReference.java
@@ -40,12 +40,6 @@ public BytesReference slice(int from, int length) {
         throw new UnsupportedOperationException("RandomBlobContentBytesReference#slice(int, int) is unsupported");
     }
 
-    @Override
-    public BytesReference copy(int from, int length) {
-        assert false : "must not copy a RandomBlobContentBytesReference";
-        throw new UnsupportedOperationException("RandomBlobContentBytesReference#copy(int, int) is unsupported");
-    }
-
     @Override
     public long ramBytesUsed() {
         // no need for accurate accounting of the overhead since we don't really account for these things anyway