On Fri, 1 Dec 2023 12:44:17 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Can I please get a review for this change to the test library's 
>> `OutputAnalyzer` class, which proposes to remove some unnecessary logging 
>> from the `getExitValue()` call?
>> 
>> As noted in https://bugs.openjdk.org/browse/JDK-8321163, right now this 
>> method logs:
>> 
>> 
>> [2023-11-24T11:47:54.557561Z] Waiting for completion for process 24909
>> [2023-11-24T11:47:54.557873Z] Waiting for completion finished for process 
>> 24909
>> 
>> 
>> even when the process has already completed and the exit value already 
>> known. The change in this PR makes it such that if the exit value is 
>> available then we no longer log this (nor call `process.waitFor()`).
>> 
>> No new tests have been added given the nature of this change. tier1, tier2 
>> and tier3 tests continue to pass with this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove micro optimization

Technically, the volatile is not enough to avoid racy writing into exitValue. 
However, it doesn't affect logic.

-------------

Marked as reviewed by lmesnik (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16919#pullrequestreview-1760491166

Reply via email to