Replace BytesRefHash and clean up alternative implementations

Signed-off-by: Ketan Verma <ketan9495@gmail.com>
opensearch-project · Jul 31, 2023 · 6ca2eb1 · 6ca2eb1
1 parent a8c07f9
commit 6ca2eb1
Show file tree

Hide file tree

Showing 14 changed files with 315 additions and 957 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -100,6 +100,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Exclude 'benchmarks' from codecov report ([#8805](https://github.com/opensearch-project/OpenSearch/pull/8805))
 - [Refactor] MediaTypeParser to MediaTypeParserRegistry ([#8636](https://github.com/opensearch-project/OpenSearch/pull/8636))
 - Create separate SourceLookup instance per segment slice in SignificantTextAggregatorFactory ([#8807](https://github.com/opensearch-project/OpenSearch/pull/8807))
+- Performance improvements for BytesRefHash ([#8788](https://github.com/opensearch-project/OpenSearch/pull/8788))
 
 ### Deprecated
 

diff --git a/benchmarks/src/main/java/org/opensearch/common/util/BytesRefHashBenchmark.java b/benchmarks/src/main/java/org/opensearch/common/util/BytesRefHashBenchmark.java
@@ -8,7 +8,6 @@
 
 package org.opensearch.common.util;
 
-import net.openhft.hashing.LongHashFunction;
 import org.apache.lucene.util.BytesRef;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -23,7 +22,6 @@
 import org.openjdk.jmh.annotations.TearDown;
 import org.openjdk.jmh.annotations.Warmup;
 import org.openjdk.jmh.infra.Blackhole;
-import org.opensearch.common.lease.Releasable;
 import org.opensearch.common.lease.Releasables;
 
 import java.util.HashSet;
@@ -32,7 +30,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 
-@Fork(value = 5)
+@Fork(value = 3)
 @Warmup(iterations = 1, time = 2)
 @Measurement(iterations = 3, time = 5)
 @BenchmarkMode(Mode.AverageTime)
@@ -45,17 +43,14 @@ public class BytesRefHashBenchmark {
     public void add(Blackhole bh, Options opts) {
         for (int hit = 0; hit < NUM_HITS; hit++) {
             BytesRef key = opts.keys[hit % opts.keys.length];
-            for (HashTable table : opts.tables) {
+            for (BytesRefHash table : opts.tables) {
                 bh.consume(table.add(key));
             }
         }
     }
 
     @State(Scope.Benchmark)
     public static class Options {
-        @Param({ "baseline", "compact", "reorganizing" })
-        public String type;
-
         @Param({
             "1",
             "2",
@@ -163,14 +158,16 @@ public static class Options {
         @Param({ "8", "32", "128" })
         public Integer length;
 
-        private HashTable[] tables;
+        private BytesRefHash[] tables;
 
         private BytesRef[] keys;
 
         @Setup
         public void setup() {
             assert size <= Math.pow(26, length) : "key length too small to generate the required number of keys";
-            tables = Stream.generate(this::newHashTable).limit(NUM_TABLES).toArray(HashTable[]::new);
+            tables = Stream.generate(() -> new BytesRefHash(BigArrays.NON_RECYCLING_INSTANCE))
+                .limit(NUM_TABLES)
+                .toArray(BytesRefHash[]::new);
             Random random = new Random(0);
             Set<BytesRef> seen = new HashSet<>();
             keys = new BytesRef[size];
@@ -193,68 +190,5 @@ public void setup() {
         public void tearDown() {
             Releasables.close(tables);
         }
-
-        private HashTable newHashTable() {
-            switch (type) {
-                case "baseline":
-                    return new HashTable() {
-                        private final BytesRefHash table = new BytesRefHash(1, 0.6f, BigArrays.NON_RECYCLING_INSTANCE);
-
-                        @Override
-                        public long add(BytesRef key) {
-                            return table.add(key);
-                        }
-
-                        @Override
-                        public void close() {
-                            table.close();
-                        }
-                    };
-                case "compact":
-                    return new HashTable() {
-                        private final CompactBytesRefHash table = new CompactBytesRefHash(
-                            1,
-                            0.6f,
-                            key -> LongHashFunction.xx3().hashBytes(key.bytes, key.offset, key.length),
-                            BigArrays.NON_RECYCLING_INSTANCE
-                        );
-
-                        @Override
-                        public long add(BytesRef key) {
-                            return table.add(key);
-                        }
-
-                        @Override
-                        public void close() {
-                            table.close();
-                        }
-                    };
-                case "reorganizing":
-                    return new HashTable() {
-                        private final ReorganizingBytesRefHash table = new ReorganizingBytesRefHash(
-                            1,
-                            0.6f,
-                            key -> LongHashFunction.xx3().hashBytes(key.bytes, key.offset, key.length),
-                            BigArrays.NON_RECYCLING_INSTANCE
-                        );
-
-                        @Override
-                        public long add(BytesRef key) {
-                            return table.add(key);
-                        }
-
-                        @Override
-                        public void close() {
-                            table.close();
-                        }
-                    };
-                default:
-                    throw new IllegalArgumentException("invalid hash table type: " + type);
-            }
-        }
-    }
-
-    private interface HashTable extends Releasable {
-        long add(BytesRef key);
     }
 }
diff --git a/libs/common/src/main/java/org/opensearch/common/Numbers.java b/libs/common/src/main/java/org/opensearch/common/Numbers.java
@@ -260,4 +260,12 @@ public static double unsignedLongToDouble(long value) {
         // want to replace that with 1 in the shifted value for correct rounding.
         return (double) ((value >>> 1) | (value & 1)) * 2.0;
     }
+
+    /**
+     * Return the strictly greater next power of two for the given value.
+     * For zero and negative numbers, it returns 1.
+     */
+    public static long nextPowerOfTwo(long value) {
+        return 1L << (Long.SIZE - Long.numberOfLeadingZeros(value));
+    }
 }