Re: [PR] Reflow computeCommonPrefixLengthAndBuildHistogram to avoid crash [lucene]
uschindler commented on code in PR #12905: URL: https://github.com/apache/lucene/pull/12905#discussion_r1429672383 ## 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"; Review Comment: Yes this works. Gradle enables Tiered compiler if Jenkins or GitHub is detected. https://github.com/apache/lucene/blob/f6582ce04830b0122de90a045f1abc595a411aa1/gradle/testing/defaults-tests.gradle#L54 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Reflow computeCommonPrefixLengthAndBuildHistogram to avoid crash [lucene]
dweiss commented on code in PR #12905: URL: https://github.com/apache/lucene/pull/12905#discussion_r1429647827 ## 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"; Review Comment: Didn't know about it either. Will this work? https://github.com/apache/lucene/pull/12953 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Reflow computeCommonPrefixLengthAndBuildHistogram to avoid crash [lucene]
uschindler commented on code in PR #12905: URL: https://github.com/apache/lucene/pull/12905#discussion_r1429628260 ## lucene/core/src/test/org/apache/lucene/util/bkd/TestDocIdsWriter.java: ## @@ -150,4 +157,28 @@ public Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { } 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); Review Comment: There's a much easier way to detect if CI is enabled. Use `assumeFalse(oal.util.Constants.IS_CLIENT_VM)` (this was added for vector API detection: https://lucene.apache.org/core/9_9_1/core/org/apache/lucene/util/Constants.html#IS_CLIENT_VM) ## 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"; Review Comment: Hi Chris, this env var isn't too useful as Jenkins does not set it. The logic to detect the CI availability is harder, but actually please remove the permission again, see below! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Reflow computeCommonPrefixLengthAndBuildHistogram to avoid crash [lucene]
ChrisHegarty merged PR #12905: URL: https://github.com/apache/lucene/pull/12905 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org