vaquarkhan commented on code in PR #17331:
URL: https://github.com/apache/iceberg/pull/17331#discussion_r3663466992
##########
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. I removed the separate --add-opens list and the JMH task
now reuses the central project.property('extraJvmArgs'), 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 it never inherited
extraJvmArgs; reusing it fixes the sun.util.calendar warm-up failure without a
second list. I also dropped sun.security.krb5 and
-XX:+IgnoreUnrecognizedVMOptions since extraJvmArgs already covers what the
benchmarks need. -PjmhHeap and -PjmhFailOnError remain the only new knobs,
defaults unchanged (32g, failOnError true). Verified on JDK 17 with ./gradlew
:iceberg-core:jmhClasses, and a full Spark read-path benchmark 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]
