vaquarkhan commented on code in PR #17331:
URL: https://github.com/apache/iceberg/pull/17331#discussion_r3663491175
##########
jmh.gradle:
##########
@@ -62,11 +62,33 @@ configure(jmhProjects) {
jmh {
jmhVersion = '1.37'
- failOnError = true
+ // Was hard-coded true; one failing param combination aborted the whole
suite and discarded
+ // already-completed results. Make it configurable, default preserved.
+ failOnError = project.hasProperty('jmhFailOnError') ?
project.property('jmhFailOnError').toBoolean() : true
forceGC = true
includeTests = true
humanOutputFile = file(jmhOutputPath)
- jvmArgs = ['-Xmx32g']
+ // Configurable heap (default preserved at 32g); full-scale
PlanningBenchmark ...WithStats OOMs at 32g.
+ // Plus the JDK 17 module opens Spark 4.x needs (sun.util.calendar for
date conversion, etc.).
+ jvmArgs = [
+ '-Xmx' + (project.findProperty('jmhHeap') ?: '32g'),
+ '-XX:+IgnoreUnrecognizedVMOptions',
+ '--add-opens=java.base/java.lang=ALL-UNNAMED',
+ '--add-opens=java.base/java.lang.invoke=ALL-UNNAMED',
+ '--add-opens=java.base/java.lang.reflect=ALL-UNNAMED',
+ '--add-opens=java.base/java.io=ALL-UNNAMED',
+ '--add-opens=java.base/java.net=ALL-UNNAMED',
+ '--add-opens=java.base/java.nio=ALL-UNNAMED',
+ '--add-opens=java.base/java.util=ALL-UNNAMED',
+ '--add-opens=java.base/java.util.concurrent=ALL-UNNAMED',
+ '--add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED',
+ '--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED',
+ '--add-opens=java.base/sun.nio.ch=ALL-UNNAMED',
+ '--add-opens=java.base/sun.nio.cs=ALL-UNNAMED',
+ '--add-opens=java.base/sun.security.action=ALL-UNNAMED',
+ '--add-opens=java.base/sun.util.calendar=ALL-UNNAMED',
+ '--add-opens=java.security.jgss/sun.security.krb5=ALL-UNNAMED'
+ ]
Review Comment:
Good point, thanks. Consolidated in fae7942: the JMH task now reuses the
central project.property('extraJvmArgs') instead of a second hand-maintained
list, so there is a single source of truth and no drift.
Root cause was that the jmh block assigned jvmArgs = ['-Xmx32g'] with =, so
unlike the test tasks (which do jvmArgs += project.property('extraJvmArgs')) it
never inherited the shared opens, which is why Spark 4.x failed at warm-up on
JDK 17 (sun.util.calendar). I also dropped the speculative sun.security.krb5
and -XX:+IgnoreUnrecognizedVMOptions, since extraJvmArgs already covers
everything the benchmarks need.
-PjmhHeap and -PjmhFailOnError remain the only new knobs, defaults unchanged
(32g, failOnError true). Verified on JDK 17: a full Spark read-path JMH
benchmark (IcebergSourceParquetEqDeleteBenchmark) ran to completion with this
harness.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
