From c4868a344b03413636d83d769ef37a00089ca158 Mon Sep 17 00:00:00 2001
From: Kiran Reddy <kkreddy@amazon.com>
Date: Mon, 28 Aug 2023 13:51:37 -0700
Subject: [PATCH] Add benchmark to measure performance of
 CustomBinaryDocValuesField

Benchmarks show that ArrayList performs better than TreeSet. Added a comment
on where to find the results and benchmark.

Signed-off-by: Kiran Reddy <kkreddy@amazon.com>
---
 .../CustomBinaryDocValuesFieldBenchmark.java  | 81 +++++++++++++++++++
 .../index/mapper/BinaryFieldMapper.java       |  4 +
 2 files changed, 85 insertions(+)
 create mode 100644 benchmarks/src/main/java/org/opensearch/benchmark/index/mapper/CustomBinaryDocValuesFieldBenchmark.java

diff --git a/benchmarks/src/main/java/org/opensearch/benchmark/index/mapper/CustomBinaryDocValuesFieldBenchmark.java b/benchmarks/src/main/java/org/opensearch/benchmark/index/mapper/CustomBinaryDocValuesFieldBenchmark.java
new file mode 100644
index 0000000000000..1f13e3a3bf2d7
--- /dev/null
+++ b/benchmarks/src/main/java/org/opensearch/benchmark/index/mapper/CustomBinaryDocValuesFieldBenchmark.java
@@ -0,0 +1,81 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.benchmark.index.mapper;
+
+import org.apache.lucene.util.BytesRef;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.opensearch.index.mapper.BinaryFieldMapper;
+
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.TimeUnit;
+
+
+@Warmup(iterations = 1)
+@Measurement(iterations = 1)
+@Fork(1)
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@State(Scope.Thread)
+@SuppressWarnings("unused") // invoked by benchmarking framework
+public class CustomBinaryDocValuesFieldBenchmark {
+
+    static final String FIELD_NAME = "dummy";
+    static final String SEED_VALUE = "seed";
+
+    @Benchmark
+    public void add(CustomBinaryDocValuesFieldBenchmark.BenchmarkParameters parameters, Blackhole blackhole) {
+        // Don't use the parameter binary doc values object.
+        // Start with a fresh object every call and add maximum number of entries
+        BinaryFieldMapper.CustomBinaryDocValuesField customBinaryDocValuesField =
+            new BinaryFieldMapper.CustomBinaryDocValuesField(FIELD_NAME, new BytesRef(SEED_VALUE).bytes);
+        for (int i = 0; i < parameters.maximumNumberOfEntries; ++i) {
+            ThreadLocalRandom.current().nextBytes(parameters.bytes);
+            customBinaryDocValuesField.add(parameters.bytes);
+        }
+    }
+
+    @Benchmark
+    public void binaryValue(CustomBinaryDocValuesFieldBenchmark.BenchmarkParameters parameters, Blackhole blackhole) {
+        blackhole.consume(parameters.customBinaryDocValuesField.binaryValue());
+    }
+
+    @State(Scope.Benchmark)
+    public static class BenchmarkParameters {
+        @Param({"8", "32", "128", "512"})
+        int maximumNumberOfEntries;
+
+        @Param({"8", "32", "128", "512"})
+        int entrySize;
+
+        BinaryFieldMapper.CustomBinaryDocValuesField customBinaryDocValuesField;
+        byte[] bytes;
+
+        @Setup
+        public void setup() {
+            customBinaryDocValuesField = new BinaryFieldMapper.CustomBinaryDocValuesField(FIELD_NAME,
+                new BytesRef(SEED_VALUE).bytes);
+            bytes = new byte[entrySize];
+            for (int i = 0; i < maximumNumberOfEntries; ++i) {
+                ThreadLocalRandom.current().nextBytes(bytes);
+                customBinaryDocValuesField.add(bytes);
+            }
+        }
+    }
+}
diff --git a/server/src/main/java/org/opensearch/index/mapper/BinaryFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/BinaryFieldMapper.java
index 62a4af247e0fa..040491f775357 100644
--- a/server/src/main/java/org/opensearch/index/mapper/BinaryFieldMapper.java
+++ b/server/src/main/java/org/opensearch/index/mapper/BinaryFieldMapper.java
@@ -242,6 +242,10 @@ protected String contentType() {
      */
     public static class CustomBinaryDocValuesField extends CustomDocValuesField {
 
+        // We considered using a TreeSet instead of an ArrayList here.
+        // Benchmarks show that ArrayList performs much better
+        // For details, see: https://github.com/opensearch-project/OpenSearch/pull/9426
+        // Benchmarks are in CustomBinaryDocValuesFiledBenchmark
         private final ArrayList<byte[]> bytesList;
 
         public CustomBinaryDocValuesField(String name, byte[] bytes) {