rmuir commented on code in PR #12311:
URL: https://github.com/apache/lucene/pull/12311#discussion_r1201265341
##########
gradle/testing/defaults-tests.gradle:
##########
@@ -119,11 +119,16 @@ allprojects {
if (rootProject.runtimeJavaVersion < JavaVersion.VERSION_16) {
jvmArgs '--illegal-access=deny'
}
-
+
// Lucene needs to optional modules at runtime, which we want to enforce
for testing
// (if the runner JVM does not support them, it will fail tests):
jvmArgs '--add-modules', 'jdk.unsupported,jdk.management'
+ // Enable the vector incubator module on supported Java versions:
+ if
(rootProject.vectorIncubatorJavaVersions.contains(rootProject.runtimeJavaVersion))
{
+ jvmArgs '--add-modules', 'jdk.incubator.vector'
+ }
+
Review Comment:
I'm not sure we should add this to jvm args by default?
Currently we run our tests with C2 disabled (like any sane person): massive
speedup to the build. Otherwise lucene's test suite is basically just a test of
C2.
CI builds (like policeman jenkins) can run by overriding JVM args so they
get C2 and different garbage collectors and all that. They could explicitly add
this safely?
Unfortunately the vector API is unusably slow without C2: like it will never
finish. Really they should just throw exception rather than fallback to
insanely slow stuff: or do something differently here. The "fallback" scalar
implementation is slower than a Galapagos turtle.
I guess to summarize, it isn't enough to just add `--add-modules
jdk.incubator.vector`, you need to turn on C2 as well or the tests will never
finish. So I think we shouldn't automagically do it?
Or maybe on our side, we can actually detect that C2 is disabled in our
provider mechanism (similar to the Locale check)? We should protect our users
and developers from the horrorshow of the "fallback" performance of the vector
api and just use scalar functions.
As a workaround, I added the following to my `~/.gradle/gradle.properties`:
```
# enables c2 (slow!) for vector stuff
tests.jvmargs=-XX:+UseParallelGC -XX:ActiveProcessorCount=1
```
It makes my tests run twice as slow for this branch but at least they finish
before our sun goes supernova, so I can run them before pushing.
--
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
