jmx-dev RFR: 8235681: Remove unnecessary workarounds in UnixOperatingSystem.c

Daniil Titov daniil.x.titov at oracle.com
Wed Jan 22 23:30:12 UTC 2020


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
    >
    >
    
    




More information about the jmx-dev mailing list