[PATCH for preview] Add version and effective JVM path to the JSON output
Aleksey Shipilev
ashipile at redhat.com
Mon Apr 17 09:39:07 UTC 2017
Hi Jens,
Sorry for late reply: conference/vacation weeks.
On 04/03/2017 05:46 AM, Jens Wilke wrote:
> I got spurious errors about wrong JavaDoc. That's why some JavaDoc fixes are sprinkled in, sorry
No worries, can be done within this patch.
> JVM version is included in the JSON file with the same print format we currently have. Example: "jvmVersion" : "JDK 1.8.0_121, VM 25.121-b13".
> This is bad. We should have different fields for it and name it java.version and java.vm.version. To have the distinct values I tend to
> change VersionMain to return the complete system properties in properties file format. Thoughts?
Yes, makes sense. VersionMain is internal anyway, and we can do whatever pleases
us with it.
> To have arbitrary strings in JSON is a major change. At the moment JMH is doing some tidying and pretty printing of the JSON via regular expressions.
> This fails for arbitrary string content. That might prove a bad trap, when there is the wrong character in a parameter. Options: Replace tidying maybe
> even writing with a library; low tech: escape all special characters in strings, tidy, then replace back. I already added commons lang3 as
> dependency to have JSON string escaping. Thoughts?
"Low tech" is my preference here.
Two comments for the patch:
> + <dependency>
> + <groupId>org.apache.commons</groupId>
> + <artifactId>commons-lang3</artifactId>
> + <version>3.5</version>
> + </dependency>
We prefer not to have external dependencies. We now reference commons-math, and
that is already questionable...
> + pw.println("\"jvm\" : \"" + StringEscapeUtils.escapeJson(params.getJvm().replace(',', ';')) + "\",");
> + // if empty, write an empty array.
> + pw.println("\"jvmArgs\" : [");
> + printStringArray(pw, params.getJvmArgs());
> + pw.println("],");
> + // FIXME: the string contains JDK and VM versions, should be separate JSON field
> + pw.println("\"jvmVersion\" : \"" + StringEscapeUtils.escapeJson(params.getJvmVersion().replace(',', ';')) + "\",");
I agree, better to split.
Thanks,
-Aleksey
More information about the jmh-dev
mailing list