Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-06 Thread Stefan Karlsson
On Wed, 6 Dec 2023 01:56:46 GMT, David Holmes wrote: > FWIW exitCode out numbers exitValue in source code 3:1 (and > 5:1 in test > code). That might be the case, but both the called function returning the value and the function we are changing uses the name exitValue. It seems irrelevant that

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-05 Thread David Holmes
On Tue, 5 Dec 2023 10:56:44 GMT, Jaikiran Pai wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> remove micro optimization > > Thank you Stefan and Leonid for the reviews. Sorry I missed this party :) Thanks for

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-05 Thread Jaikiran Pai
On Fri, 1 Dec 2023 12:44:17 GMT, Jaikiran Pai 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

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-01 Thread Jaikiran Pai
On Fri, 1 Dec 2023 20:01:42 GMT, Leonid Mesnik wrote: > Technically, the volatile is not enough to avoid racy writing into exitValue. > However, it doesn't affect logic. Agreed. - PR Comment: https://git.openjdk.org/jdk/pull/16919#issuecomment-1836981670

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-01 Thread Leonid Mesnik
On Fri, 1 Dec 2023 12:44:17 GMT, Jaikiran Pai 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

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-01 Thread Stefan Karlsson
On Fri, 1 Dec 2023 12:44:17 GMT, Jaikiran Pai 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

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-01 Thread Jaikiran Pai
On Fri, 1 Dec 2023 12:09:14 GMT, Stefan Karlsson wrote: >> Hello Stefan, this is just for a tiny optimization to prevent multiple reads >> (in the same thread) on the `volatile` field `processExitCode` in this >> method. > > I don't think such an optimization is needed here. This is not >

Re: RFR: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed [v3]

2023-12-01 Thread Jaikiran Pai
> 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: > > >