RFR(XS): 8174768: Make ProcessTools print executed process output into a separate file

[David Holmes]

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 at 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.
>>>