From 676cf76a1cf3a2aea4e8ea4b15f57c808b60aafe Mon Sep 17 00:00:00 2001 From: iverase Date: Sun, 15 Oct 2023 20:29:24 +0200 Subject: [PATCH] Remove BytesReference#copy --- .../common/bytes/BytesArray.java | 8 ---- .../common/bytes/BytesReference.java | 9 +--- .../common/bytes/CompositeBytesReference.java | 20 --------- .../common/bytes/PagedBytesReference.java | 20 --------- .../bytes/ReleasableBytesReference.java | 6 --- .../common/io/stream/StreamInput.java | 4 +- .../common/bytes/ZeroBytesReference.java | 5 --- .../common/bytes/ZeroBytesReferenceTests.java | 17 ++------ .../indices/IndicesRequestCacheTests.java | 5 --- .../bytes/AbstractBytesReferenceTestCase.java | 41 ------------------- .../RandomBlobContentBytesReference.java | 6 --- 11 files changed, 7 insertions(+), 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 index 56da15f114c36..1e171b954aa7d 100644 --- 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 index e83924722d704..21292a92d1dc1 100644 --- 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 index fc71112329297..09ccab35d1e43 100644 --- 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 index a2a992b0d9631..8e743b1fcbcd0 100644 --- 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 index 782973181383d..337e4cd28c2b3 100644 --- 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 index 1506b5ade7bb2..bf7528c4e429a 100644 --- 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 index c10edc0d806e5..ecacc29f45164 100644 --- 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 index 3414a03f7a821..f90cb870ea22a 100644 --- 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 index 6ec57c4f6d4ec..590dc72e2a72b 100644 --- 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/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java index 00fbbdfc9f697..675db5bbd6330 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/bytes/AbstractBytesReferenceTestCase.java +++ b/test/framework/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/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/RandomBlobContentBytesReference.java b/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/RandomBlobContentBytesReference.java index a2227decbcaba..44627000a2de9 100644 --- a/x-pack/plugin/snapshot-repo-test-kit/src/main/java/org/elasticsearch/repositories/blobstore/testkit/RandomBlobContentBytesReference.java +++ b/x-pack/plugin/snapshot-repo-test-kit/src/main/java/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