+1
--alex
On 01/22/2020 19:02, Chris Plummer wrote:
> Looks good.
>
> Chris
>
> On 1/22/20 3:30 PM, Daniil Titov wrote:
>> Hi Chris,
>>
>> Please review a new version of the fix [1] that converts the check to
>> the assert.
>>
>>> Also, I'm not clear on the need for the *pkernelLoad
>>> initialization. It
>>> seems this is attempting to fix a different issue, but it's already
>>> initialized to 0 at the start of the function.
>> You are right, *pkernelLoad is already initialized on line 251. The
>> new version of
>> the webrev does not include this change.
>>
>> [1] Webrev: http://cr.openjdk.java.net/~dtitov/8235681/webrev.02/
>> [2] Bug: https://bugs.openjdk.java.net/browse/JDK-8235681
>>
>> Thank you,
>> Daniil
>>
>> On 1/21/20, 10:38 AM, "Chris Plummer" <chris.plummer at oracle.com> wrote:
>>
>> Hi Daniil,
>> Do you think it's worth changing the check to an assert instead of
>> removing it?
>> Also, I'm not clear on the need for the *pkernelLoad
>> initialization. It
>> seems this is attempting to fix a different issue, but it's already
>> initialized to 0 at the start of the function.
>> thanks,
>> Chris
>> On 1/17/20 7:10 PM, Daniil Titov wrote:
>> > Please review a change that removes unnecessary workaround in
>> UnixOperatingSystem.c.
>> >
>> > It looks as this workaround
>> >
>> > if (pticks->usedKernel < tmp.usedKernel) {
>> > kdiff = 0;
>> > }
>> >
>> > was added to compensate a missed initialization of
>> counters.jvmTicks that resulted in the new
>> > counters being compared with the garbage when
>> get_cpuload_internal(...) was called for the first time.
>> >
>> > This missed initialization was fixed in [3].
>> >
>> > Mach5 tier1-tier3 and open/test/hotspot/jtreg/containers/docker
>> tests successfully passed.
>> >
>> > [1] Webrev : http://cr.openjdk.java.net/~dtitov/8235681/webrev.01/
>> > [2] Issue: https://bugs.openjdk.java.net/browse/JDK-8235681
>> > [3] https://bugs.openjdk.java.net/browse/JDK-8226575
>> >
>> > Thank you,
>> > Daniil
>> >
>> >
>>
>>
>
>