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
On Fri, 1 Dec 2023 09:48:23 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 https://bugs.openjdk.org/browse/JDK-8321163, righ
On Fri, 1 Dec 2023 11:09:30 GMT, Stefan Karlsson 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-83211
On Fri, 1 Dec 2023 11:07:06 GMT, Stefan Karlsson 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-83211
> 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
On Fri, 1 Dec 2023 11:10:16 GMT, Stefan Karlsson wrote:
>> test/lib/jdk/test/lib/process/OutputBuffer.java line 158:
>>
>>> 156: boolean aborted = true;
>>> 157: try {
>>> 158: this.processExitCode = exitCode = p.waitFor();
>>
>> According to the `waitFor` java
On Fri, 1 Dec 2023 11:14:04 GMT, Jaikiran Pai wrote:
>> test/lib/jdk/test/lib/process/OutputBuffer.java line 150:
>>
>>> 148: @Override
>>> 149: public int getExitValue() {
>>> 150: Integer exitCode = this.processExitCode;
>>
>> Do we really need the local `exitCode` variable? Eve
> 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
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
> perf
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 https://bugs.openjdk.org/browse/JDK-8321163,
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 https://bugs.openjdk.org/browse/JDK-8321163,
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
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 https://bugs.openjdk.org/browse/JDK-8321163,
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 find
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
15 matches
Mail list logo