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.
diff --git a/gradle/testing/randomization/policies/tests.policy b/gradle/testing/randomization/policies/tests.policy
index 150fa97..ce23b31 100644
--- a/gradle/testing/randomization/policies/tests.policy
+++ b/gradle/testing/randomization/policies/tests.policy
@@ -60,6 +60,9 @@
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";
diff --git a/lucene/core/src/java/org/apache/lucene/util/MSBRadixSorter.java b/lucene/core/src/java/org/apache/lucene/util/MSBRadixSorter.java
index f471a3f..70948d3 100644
--- a/lucene/core/src/java/org/apache/lucene/util/MSBRadixSorter.java
+++ b/lucene/core/src/java/org/apache/lucene/util/MSBRadixSorter.java
@@ -213,7 +213,16 @@
*
* @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 @@
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 @@
}
}
}
+ 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;
diff --git a/lucene/core/src/java/org/apache/lucene/util/RadixSelector.java b/lucene/core/src/java/org/apache/lucene/util/RadixSelector.java
index 6a916ed..47c3ca6 100644
--- a/lucene/core/src/java/org/apache/lucene/util/RadixSelector.java
+++ b/lucene/core/src/java/org/apache/lucene/util/RadixSelector.java
@@ -198,7 +198,16 @@
*
* @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 @@
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 @@
}
}
}
+ 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;
diff --git a/lucene/core/src/test/org/apache/lucene/util/bkd/TestDocIdsWriter.java b/lucene/core/src/test/org/apache/lucene/util/bkd/TestDocIdsWriter.java
index 6610bdb..6ddcabb 100644
--- a/lucene/core/src/test/org/apache/lucene/util/bkd/TestDocIdsWriter.java
+++ b/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.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 @@
}
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);
+ }
}
