[PATCH]: Forking benchmarks with long classpath on Windows (7)
Dávid Karnok
akarnokd at gmail.com
Fri Dec 22 20:12:29 UTC 2017
Unfortunately, the system property approach doesn't work. The JMH plugin
(0.4.5) can only set the `jvmArgs*` and not the -D param on the task that
runs the parent JMH process. (I.e., it would require a change here:
https://github.com/melix/jmh-gradle-plugin/blob/19ff7a4ed013e7980ef162534e0fefc91536d12c/src/main/groovy/me/champeau/gradle/JMHTask.java#L64
)
I don't have much hope the plugin would evolve in reasonable time to
accomodate this property so I suggest adding a workaround
for checking for the flag:
String jvmargs = ""
+ options.getJvmArgs().orElse(Collections.<String>emptyList())
+ options.getJvmArgsPrepend().orElse(Collections.<String>emptyList())
+ options.getJvmArgsAppend().orElse(Collections.<String>emptyList());
if (Boolean.getBoolean("jmh.separateClasspathJAR")
|| jvmargs.contains("jmh.separateClasspathJAR=true")) {
With this change, my benchmarks executed under Gradle 4.3.1, JMH Plugin
0.4.5 and setting jvmArgs.
2017-12-22 18:56 GMT+01:00 Dávid Karnok <akarnokd at gmail.com>:
> Thanks Aleksey,
>
> The OS dependence is by no coincidence. The problem my patch tries to
> solve manifests itself on Windows only due to fundamental limits on how
> long command lines can be on that platform. I think this temp file trick is
> unnecessary on Linux/Mac.
>
> Indeed, the classpath entries could be relative paths which my patch
> unconditionally turns into incorrect paths by prepending that forward slash
> that is otherwise required by absolute paths to work (example:
> "/C:/somepath/file.jar").
>
> Yes, Gradle is the main source of the lengthy path, but it uses a similar
> technique on Java 8 to work around the Windows' limit. Unfortunately, when
> the parent JMH is run, Java collects all those classpath entries in its JAR
> file into the java.class.path string that JMH is then uses in its entirety.
> I'd say Gradle's only fault is to dump all of its JAR files when the
> initial JMH Java process is forked. The issue was brought up for Gradle and
> the JMH plugin to no avail. Gradle would likely need deep changes to
> introduce filtering and the JMH plugin has no control over the classpath
> expansion which happens *after* it due to how Gradle works.
>
> (Afaik, Java 9 started supporting the "java @file" notion in command line
> where "file" can contain any number and any length of command line options.
> Of course, JMH being Java 8+, this option doesn't work.)
>
> Your take on the patch looks okay, I'll try to test it locally through the
> JMH Gradle plugin.
>
>
>
> 2017-12-22 18:28 GMT+01:00 Aleksey Shipilev <shade at redhat.com>:
>
>> Thanks!
>>
>> So I have a few problems with this patch: it takes a completely different
>> route for one particular
>> OS, which seems odd, and complicates testing, plus (I think) it relies on
>> relative paths to
>> classpath entries being the same as the path to the generated JAR, etc.
>>
>> Here's my take:
>> http://cr.openjdk.java.net/~shade/jmh/long-classpath.patch
>>
>> Hacking this further, my lingering doubt it is a JMH place to handle this
>> kind of failure escalated
>> to a firm concern. It would seems that Gradle is responsible for adding
>> cruft to the classpath on
>> the platforms that are sensitive to this. In that light, the workaround
>> is disabled by default and
>> can enabled with -Djmh.separateClasspathJAR=true. How about you ask
>> Gradle and/or Gradle JMH plugin
>> maintainers to fix their stack, and until then use this optional
>> workaround?
>>
>> -Aleksey
>>
>>
>> On 12/21/2017 11:01 AM, Dávid Karnok wrote:
>> > Index: jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java
>> > IDEA additional info:
>> > Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
>> > <+>UTF-8
>> > ===================================================================
>> > --- jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java(revision
>> > 1434:1ddf31f810a3100b9433c3fedf24615e85b1d1a7)
>> > +++ jmh-core/src/main/java/org/openjdk/jmh/runner/Runner.java(revision
>> 1434+:1ddf31f810a3+)
>> > @@ -47,6 +47,8 @@
>> > import java.nio.channels.OverlappingFileLockException;
>> > import java.util.*;
>> > import java.util.concurrent.TimeUnit;
>> > +import java.util.jar.*;
>> > +import java.util.zip.*;
>> >
>> > /**
>> > * Runner executes JMH benchmarks.
>> > @@ -826,7 +828,34 @@
>> > // assemble final process command
>> > command.add("-cp");
>> > if (Utils.isWindows()) {
>> > - command.add('"' + System.getProperty("java.class.path") +
>> '"');
>> > + try {
>> > + File tmpFile = File.createTempFile("jmh-classpath-file",
>> ".jar");
>> > + tmpFile.deleteOnExit();
>> > +
>> > + StringBuilder pw = new StringBuilder();
>> > + for (String cp : System.getProperty("java.class.path").split(";"))
>> {
>> > + pw.append("/");
>> > + pw.append(cp.replace('\\', '/').replace(" ",
>> "%20"));
>> > + if (!cp.endsWith(".jar")) {
>> > + pw.append('/');
>> > + }
>> > + pw.append(" ");
>> > + }
>> > +
>> > + Manifest manifest = new Manifest();
>> > + Attributes attributes = manifest.getMainAttributes();
>> > + attributes.put(Attributes.Name.MANIFEST_VERSION,
>> "1.0");
>> > + attributes.putValue("Class-Path",
>> pw.toString().trim());
>> > +
>> > + try (JarOutputStream zout = new JarOutputStream(new
>> FileOutputStream(tmpFile),
>> > manifest)) {
>> > + ZipEntry ze = new ZipEntry("META-INF/");
>> > + zout.putNextEntry(ze);
>> > + }
>> > +
>> > + command.add("\"" + tmpFile.getAbsolutePath() + "\"");
>> > + } catch (IOException ex) {
>> > + command.add('"' + System.getProperty("java.class.path")
>> + '"');
>> > + }
>> > } else {
>> > command.add(System.getProperty("java.class.path"));
>> > }
>> >
>> >
>> >
>> >
>> >
>> >
>> > 2017-12-21 8:24 GMT+01:00 Aleksey Shipilev <shade at redhat.com <mailto:
>> shade at redhat.com>>:
>> >
>> > Hi David,
>> >
>> > On 12/12/2017 11:36 AM, Dávid Karnok wrote:
>> > > I'd like to submit a patch (attached) to JMH's
>> > > org.openjdk.jmh.runner.Runner.java that creates a temporary jar
>> file with
>> > > all classpath entries on Windows so that the command line doesn't
>> overflow.
>> >
>> > The patch got stripped by the mailing list, apparently. (Wrong MIME
>> type?)
>> >
>> > Thanks,
>> > -Aleksey
>> >
>> >
>> >
>> >
>> > --
>> > Best regards,
>> > David Karnok
>>
>>
>>
>
>
> --
> Best regards,
> David Karnok
>
--
Best regards,
David Karnok
More information about the jmh-dev
mailing list