[PATCH]: Forking benchmarks with long classpath on Windows (7)

[Dávid Karnok]

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