Thanks to Igor Ignatyev, a new version is available at http://cr.openjdk.java.net/~iignatyev/enikitin/8174768/webrev.02 Regards, Evgeny. On 2020-04-08 17:19, Igor Ignatev wrote:
You can use %n in printf to get a system-specific newline.
— Igor
On Apr 8, 2020, at 8:12 AM, Evgeny Nikitin <evgeny.nikitin@oracle.com> wrote:
Hi David,
You can use printf rather than making a separate call to format. Good point, I've missed that method. I'll fix it and ask Igor to change the webrev.
I finally decided to leave it as it is now, since printf doesn't add a newline and System.lineSeparator() is not shorter than String.format() :). Please let me know if you disagree.
Best regards, Evgeny.
On 2020-04-07 16:09, Evgeny Nikitin wrote: Hi David,
why did you introduce a new block scope? To separate the action block from the other code. "A Poor/lazy man's method". I've preferred to lay it out this way because it makes the code more compact and easy to read (as opposing to looking for a only-once used method in some other part of the file) and due to the confusing name of OutputAnalyzer type (which I need as a storage for output, not as some analyzer). Creating a method with OutputAnalyzer as a parameter would make this method's purpose even more confusing. You can use printf rather than making a separate call to format. Good point, I've missed that method. I'll fix it and ask Igor to change the webrev. Regards, Evgeny. On 2020-04-07 14:50, David Holmes wrote: Hi Evgeny,
On 7/04/2020 6:33 pm, Evgeny Nikitin wrote:
Hi David,
I'm not at all sure this is generally what we want for every single test that uses ProcessTools! But I'm willing it to see it trialed.
Well, it's mostly used for test VM runs. In runs I performed (artificial "failures" included) the amounts of output were very small.
Please run full tier testing at least to tier 6 and ideally beyond before pushing this. There are potential implications for temporary (and more permanent) disk usage as well as additional time needed to write files out to disk. (Hopefully these are generally small enough that this doesn't make a noticeable difference.) Done, thanks for suggestion. The additional runs showed no problems.
Good to know - thanks.
I've provided Igor with a slightly modified version (Added notification about the output file into the main test's log). Please review:
http://cr.openjdk.java.net/~iignatyev/enikitin/8174768/webrev.01/
A couple of minor nits:
397 {
why did you introduce a new block scope?
404 System.out.println(String.format(
You can use printf rather than making a separate call to format.
No need to see any update if you chose to make it.
Thanks, David -----
Best Regards,
Evgeny.
On 2020-04-02 02:07, David Holmes wrote:
Thanks for sharing this Igor!
I'm not at all sure this is generally what we want for every single test that uses ProcessTools! But I'm willing it to see it trialed.
Evgeny: Please run full tier testing at least to tier 6 and ideally beyond before pushing this. There are potential implications for temporary (and more permanent) disk usage as well as additional time needed to write files out to disk. (Hopefully these are generally small enough that this doesn't make a noticeable difference.)
Thanks, David
On 2/04/2020 5:13 am, Igor Ignatyev wrote: > Hi Evgeny, > > (widening the audience, given this affects not just hotspot compiler, but hotspot tests as well as core libs tests in general) > > overall that looks good to me. one suggestion, for the ease of failure analysis it might be worth to print out the names of created files, although this might potentially clutter the output, I don't think it'll be a problem given we already print out things like 'Gathering output for process ...' , 'Waiting for completion...' in LazyOutputBuffer. > >> The change has been tested via a mach5 test runs (jdk-tier1 through 4) on the 4 common platforms (linux-x64, windows-x64, macosx-x64, sparcv9). > this doesn't include any of hotspot tiers, could you please also run hs-tier1--4? > // you can use tierN jobs which include both jdk and hs parts. > > Thanks, > -- Igor > >> On Mar 30, 2020, at 3:55 AM, Evgeny Nikitin <evgeny.nikitin@oracle.com> wrote: >> >> >> Hi, >> >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8174768 >> >> Webrev: http://cr.openjdk.java.net/~iignatyev/enikitin/8174768/webrev.00/ >> >> >> The bug had been created as a request to simplify investigation for compiler control tests failures. >> I found the functionality pretty generic and useful and made ProcessTools dump output as well as some diagnostic information for every executed process into a separate file. >> The diagnostic information contains cmdline, exit code, stdout and stderr. The output files are named like 'pid-<PID>-output.log'. >> >> The change has been tested via a mach5 test runs (jdk-tier1 through 4) on the 4 common platforms (linux-x64, windows-x64, macosx-x64, sparcv9). >> >> Please review, >> /Evgeny Nikitin. >