From 5ef4e81c7ecccf4ee58dc2dbccb07c2a978b0562 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 19 Jun 2024 15:10:46 +0200 Subject: [PATCH] Add getAndSet to some BigArray implementations and make set return void (#109878) The majority of callsites for these things don't look at the return. Computing the return from bytes to numeric values is not free due to bounds checks etc. => this PR adds a set that doesn't read the previous value and where needed moves the existing code to a new getAndSet call --- .../common/util/AbstractHash.java | 2 +- .../elasticsearch/common/util/BigArrays.java | 28 ++++++++++------- .../common/util/BigByteArray.java | 4 +-- .../common/util/BigDoubleArray.java | 4 +-- .../common/util/BigFloatArray.java | 4 +-- .../common/util/BigIntArray.java | 9 +++++- .../common/util/BigLongArray.java | 10 ++++++- .../elasticsearch/common/util/ByteArray.java | 4 +-- .../common/util/DoubleArray.java | 4 +-- .../elasticsearch/common/util/FloatArray.java | 4 +-- .../elasticsearch/common/util/Int3Hash.java | 6 ++-- .../elasticsearch/common/util/IntArray.java | 7 ++++- .../elasticsearch/common/util/LongArray.java | 7 ++++- .../elasticsearch/common/util/LongHash.java | 2 +- .../common/util/LongLongHash.java | 4 +-- .../common/util/ReleasableByteArray.java | 2 +- .../common/util/ReleasableDoubleArray.java | 2 +- .../common/util/ReleasableIntArray.java | 7 ++++- .../common/util/ReleasableLongArray.java | 7 ++++- .../GlobalOrdinalsStringTermsAggregator.java | 2 +- .../common/util/MockBigArrays.java | 30 ++++++++++++------- 21 files changed, 97 insertions(+), 52 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/util/AbstractHash.java b/server/src/main/java/org/elasticsearch/common/util/AbstractHash.java index b0dc6d98fe16b..2bcc9b48ff1b8 100644 --- a/server/src/main/java/org/elasticsearch/common/util/AbstractHash.java +++ b/server/src/main/java/org/elasticsearch/common/util/AbstractHash.java @@ -32,7 +32,7 @@ public long id(long index) { } protected final long id(long index, long id) { - return ids.set(index, id + 1) - 1; + return ids.getAndSet(index, id + 1) - 1; } @Override diff --git a/server/src/main/java/org/elasticsearch/common/util/BigArrays.java b/server/src/main/java/org/elasticsearch/common/util/BigArrays.java index 1e8b0cc83eaa6..199eaa83a2da3 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigArrays.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigArrays.java @@ -118,11 +118,9 @@ public byte get(long index) { } @Override - public byte set(long index, byte value) { + public void set(long index, byte value) { assert indexIsInt(index); - final byte ret = array[(int) index]; array[(int) index] = value; - return ret; } @Override @@ -215,13 +213,19 @@ public int get(long index) { } @Override - public int set(long index, int value) { + public int getAndSet(long index, int value) { assert index >= 0 && index < size(); final int ret = (int) VH_PLATFORM_NATIVE_INT.get(array, (int) index << 2); VH_PLATFORM_NATIVE_INT.set(array, (int) index << 2, value); return ret; } + @Override + public void set(long index, int value) { + assert index >= 0 && index < size(); + VH_PLATFORM_NATIVE_INT.set(array, (int) index << 2, value); + } + @Override public int increment(long index, int inc) { assert index >= 0 && index < size(); @@ -272,13 +276,19 @@ public long get(long index) { } @Override - public long set(long index, long value) { + public long getAndSet(long index, long value) { assert index >= 0 && index < size(); final long ret = (long) VH_PLATFORM_NATIVE_LONG.get(array, (int) index << 3); VH_PLATFORM_NATIVE_LONG.set(array, (int) index << 3, value); return ret; } + @Override + public void set(long index, long value) { + assert index >= 0 && index < size(); + VH_PLATFORM_NATIVE_LONG.set(array, (int) index << 3, value); + } + @Override public long increment(long index, long inc) { assert index >= 0 && index < size(); @@ -336,11 +346,9 @@ public double get(long index) { } @Override - public double set(long index, double value) { + public void set(long index, double value) { assert index >= 0 && index < size(); - final double ret = (double) VH_PLATFORM_NATIVE_DOUBLE.get(array, (int) index << 3); VH_PLATFORM_NATIVE_DOUBLE.set(array, (int) index << 3, value); - return ret; } @Override @@ -400,11 +408,9 @@ public float get(long index) { } @Override - public float set(long index, float value) { + public void set(long index, float value) { assert index >= 0 && index < size(); - final float ret = (float) VH_PLATFORM_NATIVE_FLOAT.get(array, (int) index << 2); VH_PLATFORM_NATIVE_FLOAT.set(array, (int) index << 2, value); - return ret; } @Override diff --git a/server/src/main/java/org/elasticsearch/common/util/BigByteArray.java b/server/src/main/java/org/elasticsearch/common/util/BigByteArray.java index 61848769e661d..1e714f122d885 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigByteArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigByteArray.java @@ -47,13 +47,11 @@ public byte get(long index) { } @Override - public byte set(long index, byte value) { + public void set(long index, byte value) { final int pageIndex = pageIndex(index); final int indexInPage = indexInPage(index); final byte[] page = getPageForWriting(pageIndex); - final byte ret = page[indexInPage]; page[indexInPage] = value; - return ret; } @Override diff --git a/server/src/main/java/org/elasticsearch/common/util/BigDoubleArray.java b/server/src/main/java/org/elasticsearch/common/util/BigDoubleArray.java index 27dc454c85adf..3135ebb293070 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigDoubleArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigDoubleArray.java @@ -42,13 +42,11 @@ public double get(long index) { } @Override - public double set(long index, double value) { + public void set(long index, double value) { final int pageIndex = pageIndex(index); final int indexInPage = indexInPage(index); final byte[] page = getPageForWriting(pageIndex); - final double ret = (double) VH_PLATFORM_NATIVE_DOUBLE.get(page, indexInPage << 3); VH_PLATFORM_NATIVE_DOUBLE.set(page, indexInPage << 3, value); - return ret; } @Override diff --git a/server/src/main/java/org/elasticsearch/common/util/BigFloatArray.java b/server/src/main/java/org/elasticsearch/common/util/BigFloatArray.java index 9502950c1d25b..380b2c8e12b34 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigFloatArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigFloatArray.java @@ -30,13 +30,11 @@ final class BigFloatArray extends AbstractBigByteArray implements FloatArray { } @Override - public float set(long index, float value) { + public void set(long index, float value) { final int pageIndex = pageIndex(index); final int indexInPage = indexInPage(index); final byte[] page = getPageForWriting(pageIndex); - final float ret = (float) VH_PLATFORM_NATIVE_FLOAT.get(page, indexInPage << 2); VH_PLATFORM_NATIVE_FLOAT.set(page, indexInPage << 2, value); - return ret; } @Override diff --git a/server/src/main/java/org/elasticsearch/common/util/BigIntArray.java b/server/src/main/java/org/elasticsearch/common/util/BigIntArray.java index 4388cc2308905..9ce9842c337c0 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigIntArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigIntArray.java @@ -46,7 +46,7 @@ public int get(long index) { } @Override - public int set(long index, int value) { + public int getAndSet(long index, int value) { final int pageIndex = pageIndex(index); final int indexInPage = indexInPage(index); final byte[] page = getPageForWriting(pageIndex); @@ -55,6 +55,13 @@ public int set(long index, int value) { return ret; } + @Override + public void set(long index, int value) { + final int pageIndex = pageIndex(index); + final int indexInPage = indexInPage(index); + VH_PLATFORM_NATIVE_INT.set(getPageForWriting(pageIndex), indexInPage << 2, value); + } + @Override public int increment(long index, int inc) { final int pageIndex = pageIndex(index); diff --git a/server/src/main/java/org/elasticsearch/common/util/BigLongArray.java b/server/src/main/java/org/elasticsearch/common/util/BigLongArray.java index f0ccea26880c4..7d23e06f87658 100644 --- a/server/src/main/java/org/elasticsearch/common/util/BigLongArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/BigLongArray.java @@ -41,7 +41,7 @@ public long get(long index) { } @Override - public long set(long index, long value) { + public long getAndSet(long index, long value) { final int pageIndex = pageIndex(index); final int indexInPage = indexInPage(index); final byte[] page = getPageForWriting(pageIndex); @@ -50,6 +50,14 @@ public long set(long index, long value) { return ret; } + @Override + public void set(long index, long value) { + final int pageIndex = pageIndex(index); + final int indexInPage = indexInPage(index); + final byte[] page = getPageForWriting(pageIndex); + VH_PLATFORM_NATIVE_LONG.set(page, indexInPage << 3, value); + } + @Override public long increment(long index, long inc) { final int pageIndex = pageIndex(index); diff --git a/server/src/main/java/org/elasticsearch/common/util/ByteArray.java b/server/src/main/java/org/elasticsearch/common/util/ByteArray.java index cb2b10632d08b..2c16e730635f8 100644 --- a/server/src/main/java/org/elasticsearch/common/util/ByteArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/ByteArray.java @@ -32,9 +32,9 @@ static ByteArray readFrom(StreamInput in) throws IOException { byte get(long index); /** - * Set a value at the given index and return the previous value. + * Set a value at the given index. */ - byte set(long index, byte value); + void set(long index, byte value); /** * Get a reference to a slice. diff --git a/server/src/main/java/org/elasticsearch/common/util/DoubleArray.java b/server/src/main/java/org/elasticsearch/common/util/DoubleArray.java index dde1157c905c7..80348d3b2945f 100644 --- a/server/src/main/java/org/elasticsearch/common/util/DoubleArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/DoubleArray.java @@ -28,9 +28,9 @@ static DoubleArray readFrom(StreamInput in) throws IOException { double get(long index); /** - * Set a value at the given index and return the previous value. + * Set a value at the given index. */ - double set(long index, double value); + void set(long index, double value); /** * Increment value at the given index by inc and return the value. diff --git a/server/src/main/java/org/elasticsearch/common/util/FloatArray.java b/server/src/main/java/org/elasticsearch/common/util/FloatArray.java index 33427299fe26c..057f51f45b1f6 100644 --- a/server/src/main/java/org/elasticsearch/common/util/FloatArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/FloatArray.java @@ -19,9 +19,9 @@ public interface FloatArray extends BigArray { float get(long index); /** - * Set a value at the given index and return the previous value. + * Set a value at the given index. */ - float set(long index, float value); + void set(long index, float value); /** * Fill slots between fromIndex inclusive to toIndex exclusive with value. diff --git a/server/src/main/java/org/elasticsearch/common/util/Int3Hash.java b/server/src/main/java/org/elasticsearch/common/util/Int3Hash.java index 051dd31ce8869..d9beea76b371a 100644 --- a/server/src/main/java/org/elasticsearch/common/util/Int3Hash.java +++ b/server/src/main/java/org/elasticsearch/common/util/Int3Hash.java @@ -132,9 +132,9 @@ protected void removeAndAdd(long index) { final long id = id(index, -1); assert id >= 0; long keyOffset = id * 3; - final int key1 = keys.set(keyOffset, 0); - final int key2 = keys.set(keyOffset + 1, 0); - final int key3 = keys.set(keyOffset + 2, 0); + final int key1 = keys.getAndSet(keyOffset, 0); + final int key2 = keys.getAndSet(keyOffset + 1, 0); + final int key3 = keys.getAndSet(keyOffset + 2, 0); reset(key1, key2, key3, id); } diff --git a/server/src/main/java/org/elasticsearch/common/util/IntArray.java b/server/src/main/java/org/elasticsearch/common/util/IntArray.java index 06975ffba46da..4f4dd61863595 100644 --- a/server/src/main/java/org/elasticsearch/common/util/IntArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/IntArray.java @@ -29,7 +29,12 @@ static IntArray readFrom(StreamInput in) throws IOException { /** * Set a value at the given index and return the previous value. */ - int set(long index, int value); + int getAndSet(long index, int value); + + /** + * Set a value at the given index + */ + void set(long index, int value); /** * Increment value at the given index by inc and return the value. diff --git a/server/src/main/java/org/elasticsearch/common/util/LongArray.java b/server/src/main/java/org/elasticsearch/common/util/LongArray.java index 59321d1957f4d..cff8c86eef4b6 100644 --- a/server/src/main/java/org/elasticsearch/common/util/LongArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/LongArray.java @@ -30,7 +30,12 @@ static LongArray readFrom(StreamInput in) throws IOException { /** * Set a value at the given index and return the previous value. */ - long set(long index, long value); + long getAndSet(long index, long value); + + /** + * Set a value at the given index. + */ + void set(long index, long value); /** * Increment value at the given index by inc and return the value. diff --git a/server/src/main/java/org/elasticsearch/common/util/LongHash.java b/server/src/main/java/org/elasticsearch/common/util/LongHash.java index 6ca4d9f0986f6..32364f8d2f341 100644 --- a/server/src/main/java/org/elasticsearch/common/util/LongHash.java +++ b/server/src/main/java/org/elasticsearch/common/util/LongHash.java @@ -110,7 +110,7 @@ public long add(long key) { protected void removeAndAdd(long index) { final long id = id(index, -1); assert id >= 0; - final long key = keys.set(id, 0); + final long key = keys.getAndSet(id, 0); reset(key, id); } diff --git a/server/src/main/java/org/elasticsearch/common/util/LongLongHash.java b/server/src/main/java/org/elasticsearch/common/util/LongLongHash.java index 13405d491298c..61dd3b457029c 100644 --- a/server/src/main/java/org/elasticsearch/common/util/LongLongHash.java +++ b/server/src/main/java/org/elasticsearch/common/util/LongLongHash.java @@ -134,8 +134,8 @@ protected void removeAndAdd(long index) { final long id = id(index, -1); assert id >= 0; long keyOffset = id * 2; - final long key1 = keys.set(keyOffset, 0); - final long key2 = keys.set(keyOffset + 1, 0); + final long key1 = keys.getAndSet(keyOffset, 0); + final long key2 = keys.getAndSet(keyOffset + 1, 0); reset(key1, key2, id); } diff --git a/server/src/main/java/org/elasticsearch/common/util/ReleasableByteArray.java b/server/src/main/java/org/elasticsearch/common/util/ReleasableByteArray.java index ce0f5bdfedd40..0eea3443391c1 100644 --- a/server/src/main/java/org/elasticsearch/common/util/ReleasableByteArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/ReleasableByteArray.java @@ -62,7 +62,7 @@ public boolean get(long index, int len, BytesRef ref) { } @Override - public byte set(long index, byte value) { + public void set(long index, byte value) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/common/util/ReleasableDoubleArray.java b/server/src/main/java/org/elasticsearch/common/util/ReleasableDoubleArray.java index 61b2f52ee384e..d7279b845f225 100644 --- a/server/src/main/java/org/elasticsearch/common/util/ReleasableDoubleArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/ReleasableDoubleArray.java @@ -44,7 +44,7 @@ public double get(long index) { } @Override - public double set(long index, double value) { + public void set(long index, double value) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/common/util/ReleasableIntArray.java b/server/src/main/java/org/elasticsearch/common/util/ReleasableIntArray.java index 2b433f6812a87..9dbc11328974a 100644 --- a/server/src/main/java/org/elasticsearch/common/util/ReleasableIntArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/ReleasableIntArray.java @@ -44,7 +44,12 @@ public int get(long index) { } @Override - public int set(long index, int value) { + public int getAndSet(long index, int value) { + throw new UnsupportedOperationException(); + } + + @Override + public void set(long index, int value) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/common/util/ReleasableLongArray.java b/server/src/main/java/org/elasticsearch/common/util/ReleasableLongArray.java index 2980713e2e652..4f36cdc890d78 100644 --- a/server/src/main/java/org/elasticsearch/common/util/ReleasableLongArray.java +++ b/server/src/main/java/org/elasticsearch/common/util/ReleasableLongArray.java @@ -45,7 +45,12 @@ public long get(long index) { } @Override - public long set(long index, long value) { + public long getAndSet(long index, long value) { + throw new UnsupportedOperationException(); + } + + @Override + public void set(long index, long value) { throw new UnsupportedOperationException(); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java index acdb24b9109af..26204e1a2530f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java @@ -380,7 +380,7 @@ private void mapSegmentCountsToGlobalCounts(LongUnaryOperator mapping) throws IO for (long i = 1; i < segmentDocCounts.size(); i++) { // We use set(...) here, because we need to reset the slow to 0. // segmentDocCounts get reused over the segments and otherwise counts would be too high. - long inc = segmentDocCounts.set(i, 0); + long inc = segmentDocCounts.getAndSet(i, 0); if (inc == 0) { continue; } diff --git a/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java b/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java index b1eddf927d3f3..9378b51de78df 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java +++ b/test/framework/src/main/java/org/elasticsearch/common/util/MockBigArrays.java @@ -389,8 +389,8 @@ public byte get(long index) { } @Override - public byte set(long index, byte value) { - return in.set(index, value); + public void set(long index, byte value) { + in.set(index, value); } @Override @@ -469,8 +469,13 @@ public int get(long index) { } @Override - public int set(long index, int value) { - return in.set(index, value); + public int getAndSet(long index, int value) { + return in.getAndSet(index, value); + } + + @Override + public void set(long index, int value) { + in.set(index, value); } @Override @@ -524,8 +529,13 @@ public long get(long index) { } @Override - public long set(long index, long value) { - return in.set(index, value); + public long getAndSet(long index, long value) { + return in.getAndSet(index, value); + } + + @Override + public void set(long index, long value) { + in.set(index, value); } @Override @@ -584,8 +594,8 @@ public float get(long index) { } @Override - public float set(long index, float value) { - return in.set(index, value); + public void set(long index, float value) { + in.set(index, value); } @Override @@ -629,8 +639,8 @@ public double get(long index) { } @Override - public double set(long index, double value) { - return in.set(index, value); + public void set(long index, double value) { + in.set(index, value); } @Override