+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.plum...@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
>
>
