Reflow computeCommonPrefixLengthAndBuildHistogram to avoid crash (#12905)

This commit reflows the code in the method body of computeCommonPrefixLengthAndBuildHistogram, so as to avoid a JVM JIT crash. The purpose of this change is to workaround the JVM bug which is somewhat fragile, but the best that we can do for now and appears to be working well.

gradle/testing/randomization/policies/tests.policy

@@ -60,6 +60,9 @@ grant {
   permission java.lang.RuntimePermission "getFileStoreAttributes";
   permission java.lang.RuntimePermission "writeFileDescriptor";
 
+  // needed to check if C2 (implied by the presence of the CI env) is enabled
+  permission java.lang.RuntimePermission "getenv.CI";
+
   // TestLockFactoriesMultiJVM opens a random port on 127.0.0.1 (port 0 = ephemeral port range):
   permission java.net.SocketPermission "127.0.0.1:0", "accept,listen,resolve";
 

lucene/core/src/java/org/apache/lucene/util/MSBRadixSorter.java

@@ -213,7 +213,16 @@ public abstract class MSBRadixSorter extends Sorter {
    *
    * @see #buildHistogram
    */
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  // See https://github.com/apache/lucene/issues/12898
   private int computeCommonPrefixLengthAndBuildHistogram(int from, int to, int k, int[] histogram) {
+    int commonPrefixLength = computeInitialCommonPrefixLength(from, k);
+    return computeCommonPrefixLengthAndBuildHistogramPart1(
+        from, to, k, histogram, commonPrefixLength);
+  }
+
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeInitialCommonPrefixLength(int from, int k) {
     final int[] commonPrefix = this.commonPrefix;
     int commonPrefixLength = Math.min(commonPrefix.length, maxLength - k);
     for (int j = 0; j < commonPrefixLength; ++j) {
@@ -224,7 +233,13 @@ public abstract class MSBRadixSorter extends Sorter {
         break;
       }
     }
+    return commonPrefixLength;
+  }
 
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeCommonPrefixLengthAndBuildHistogramPart1(
+      int from, int to, int k, int[] histogram, int commonPrefixLength) {
+    final int[] commonPrefix = this.commonPrefix;
     int i;
     outer:
     for (i = from + 1; i < to; ++i) {
@@ -239,7 +254,13 @@ public abstract class MSBRadixSorter extends Sorter {
         }
       }
     }
+    return computeCommonPrefixLengthAndBuildHistogramPart2(
+        from, to, k, histogram, commonPrefixLength, i);
+  }
 
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeCommonPrefixLengthAndBuildHistogramPart2(
+      int from, int to, int k, int[] histogram, int commonPrefixLength, int i) {
     if (i < to) {
       // the loop got broken because there is no common prefix
       assert commonPrefixLength == 0;

lucene/core/src/java/org/apache/lucene/util/RadixSelector.java

@@ -198,7 +198,16 @@ public abstract class RadixSelector extends Selector {
    *
    * @see #buildHistogram
    */
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  // See https://github.com/apache/lucene/issues/12898
   private int computeCommonPrefixLengthAndBuildHistogram(int from, int to, int k, int[] histogram) {
+    int commonPrefixLength = computeInitialCommonPrefixLength(from, k);
+    return computeCommonPrefixLengthAndBuildHistogramPart1(
+        from, to, k, histogram, commonPrefixLength);
+  }
+
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeInitialCommonPrefixLength(int from, int k) {
     final int[] commonPrefix = this.commonPrefix;
     int commonPrefixLength = Math.min(commonPrefix.length, maxLength - k);
     for (int j = 0; j < commonPrefixLength; ++j) {
@@ -209,7 +218,13 @@ public abstract class RadixSelector extends Selector {
         break;
       }
     }
+    return commonPrefixLength;
+  }
 
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeCommonPrefixLengthAndBuildHistogramPart1(
+      int from, int to, int k, int[] histogram, int commonPrefixLength) {
+    final int[] commonPrefix = this.commonPrefix;
     int i;
     outer:
     for (i = from + 1; i < to; ++i) {
@@ -226,7 +241,13 @@ public abstract class RadixSelector extends Selector {
         }
       }
     }
+    return computeCommonPrefixLengthAndBuildHistogramPart2(
+        from, to, k, histogram, commonPrefixLength, i);
+  }
 
+  // This method, and its namesakes, have been manually split to work around a JVM crash.
+  private int computeCommonPrefixLengthAndBuildHistogramPart2(
+      int from, int to, int k, int[] histogram, int commonPrefixLength, int i) {
     if (i < to) {
       // the loop got broken because there is no common prefix
       assert commonPrefixLength == 0;

lucene/core/src/test/org/apache/lucene/util/bkd/TestDocIdsWriter.java

@@ -17,8 +17,14 @@
 package org.apache.lucene.util.bkd;
 
 import java.io.IOException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
+import org.apache.lucene.document.IntPoint;
+import org.apache.lucene.document.SortedNumericDocValuesField;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.PointValues.IntersectVisitor;
 import org.apache.lucene.index.PointValues.Relation;
 import org.apache.lucene.store.Directory;
@@ -28,6 +34,7 @@ import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.tests.util.LuceneTestCase;
 import org.apache.lucene.tests.util.TestUtil;
 import org.apache.lucene.util.CollectionUtil;
+import org.apache.lucene.util.SuppressForbidden;
 
 public class TestDocIdsWriter extends LuceneTestCase {
 
@@ -150,4 +157,28 @@ public class TestDocIdsWriter extends LuceneTestCase {
     }
     dir.deleteFile("tmp");
   }
+
+  // This simple test tickles a JVM C2 JIT crash on JDK's less than 21.0.1
+  // Crashes only when run with C2, so with the environment variable `CI` set
+  // Regardless of whether C2 is enabled or not, the test should never fail.
+  public void testCrash() throws IOException {
+    assumeTrue("Requires C2, which is only enabled when CI env is set", getCIEnv() != null);
+    int itrs = atLeast(100);
+    for (int i = 0; i < itrs; i++) {
+      try (Directory dir = newDirectory();
+          IndexWriter iw = new IndexWriter(dir, newIndexWriterConfig(null))) {
+        for (int d = 0; d < 20_000; d++) {
+          iw.addDocument(
+              List.of(new IntPoint("foo", 0), new SortedNumericDocValuesField("bar", 0)));
+        }
+      }
+    }
+  }
+
+  @SuppressForbidden(reason = "needed to check if C2 is enabled")
+  @SuppressWarnings("removal")
+  private static String getCIEnv() {
+    PrivilegedAction<String> pa = () -> System.getenv("CI");
+    return AccessController.doPrivileged(pa);
+  }
 }