From dd006f56f7159246f98551c0c1ce33d6e461b523 Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Sat, 21 Sep 2024 12:31:43 +0200
Subject: [PATCH] Fix contention in StringHelper.randomId

Seeing this mutex contended up to O(10ms) in Elasticsearch
at times. Moving to CAS and removing the unnecessary alloction
of a new instance for the bitwise-and with the mask makes
this perform+scale much better.
---
 .../org/apache/lucene/util/StringHelper.java  | 27 +++++++++----------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/lucene/core/src/java/org/apache/lucene/util/StringHelper.java b/lucene/core/src/java/org/apache/lucene/util/StringHelper.java
index d264c1da58d..7c973254c88 100644
--- a/lucene/core/src/java/org/apache/lucene/util/StringHelper.java
+++ b/lucene/core/src/java/org/apache/lucene/util/StringHelper.java
@@ -22,6 +22,7 @@ import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Methods for manipulating strings.
@@ -210,16 +211,9 @@ public abstract class StringHelper {
   }
 
   // Holds 128 bit unsigned value:
-  private static BigInteger nextId;
-  private static final BigInteger mask128;
-  private static final Object idLock = new Object();
+  private static final AtomicReference<BigInteger> nextId;
 
   static {
-    // 128 bit unsigned mask
-    byte[] maskBytes128 = new byte[16];
-    Arrays.fill(maskBytes128, (byte) 0xff);
-    mask128 = new BigInteger(1, maskBytes128);
-
     String prop = System.getProperty("tests.seed");
 
     // State for xorshift128:
@@ -287,7 +281,7 @@ public abstract class StringHelper {
     BigInteger unsignedX1 = BigInteger.valueOf(x1).and(mask64);
 
     // Concatentate bits of x0 and x1, as unsigned 128 bit integer:
-    nextId = unsignedX0.shiftLeft(64).or(unsignedX1);
+    nextId = new AtomicReference<>(unsignedX0.shiftLeft(64).or(unsignedX1));
   }
 
   /** length in bytes of an ID */
@@ -310,12 +304,15 @@ public abstract class StringHelper {
     //     what impact that has on the period, whereas the simple ++ (mod 2^128)
     //     we use here is guaranteed to have the full period.
 
-    byte[] bits;
-    synchronized (idLock) {
-      bits = nextId.toByteArray();
-      nextId = nextId.add(BigInteger.ONE).and(mask128);
-    }
-
+    BigInteger current;
+    BigInteger next;
+    do {
+      current = nextId.get();
+      next = current.add(BigInteger.ONE);
+      // 128bit value -> unsigned overflow once the 129th bit at index 128 is set
+      next = next.testBit(128) ? BigInteger.ZERO : next;
+    } while (nextId.weakCompareAndSetVolatile(current, next) == false);
+    byte[] bits = current.toByteArray();
     // toByteArray() always returns a sign bit, so it may require an extra byte (always zero)
     if (bits.length > ID_LENGTH) {
       assert bits.length == ID_LENGTH + 1;