Resending with the corrected title, "RFR" was somehow stripped from it that 
breaks the sorting by subject...

    Hi Bob,
    
    >>    It’s too bad getCpuLoad can’t detect that the cpuset is identical to 
the hosts in order to allow you to fallback to
    >>   getSystemCpuLoad0.
    
    I think we can detect that the cpuset is identical to the host's one by 
comparing the length of the array containerMetrics.getEffectiveCpuSetCpus() 
returns
    to the number of the CPUs configured on the host and returned by  
sysconf(_SC_NPROCESSORS_CONF) . The latter could be retrieved by adding new 
native
     method to OperatingSystemImpl getConfiguredCpuCount0. If they match then 
we just fallback to getSystemCpuLoad0(). I did some testing on Linux host and
     inside Docker container with different '--cpuset-cpus' settings and it 
seems to work as expected.
    
    JNIEXPORT jint JNICALL
    Java_com_sun_management_internal_OperatingSystemImpl_getConfiguredCpuCount0
    (JNIEnv *env, jobject mbean)
    {
      if(perfInit() == 0) {
        return counters.nProcs;
      } else {
        return -1;
      }
    }
    
    
    If there is no objection I will include this change in the new webrev.
    
    Thank you,
    Daniil
    
    On 12/3/19, 1:30 PM, "Bob Vandette" <bob.vande...@oracle.com> wrote:
    
        Daniil,
        
        Looks good to me.
        
        If there are any management jtreg tests, I’d run these since your 
changes to OperatingSystemMXBean will 
        alter the behavior of these methods even for Linux hosts since cgroups 
is typically enabled causing the
        container detection to report containerized.
        
        It’s too bad getCpuLoad can’t detect that the cpuset is identical to 
the hosts in order to allow you to fallback to
        getSystemCpuLoad0. 
        
        
        Bob.
        
        
        > On Dec 3, 2019, at 2:42 PM, Daniil Titov <daniil.x.ti...@oracle.com> 
wrote:
        > 
        > Please review the change that makes OperatingSystemMXBean methods 
return container specific information
        > rather than the host based data.
        > 
        > The webrev [1] is based on the code Andrew and Severin initially 
provided with some additional changes and combined
        > with the spec update David made [3].
        > 
        > The webrev corrects the implementation for the free/total swap 
methods as Bob noted to subtract the total
        > and free memory from the returned values.
        > 
        > It also corrects getCpuLoad() implementation, as Bob advised, to 
cover the case when CPU quotas are not active.
        > 
        > The webrev also takes into account the case when 
java.security.AccessControlException exception is thrown
        > during the initialization of the container subsystem ( e.g.  when 
java.policy doesn’t grant "read" access to
        > "/proc/self/mountinfo" file).
        > 
        > CSR for the spec changes [3] is approved.
        > 
        > Testing: Mach5 tiers1, tiers2, tiers3, tier4, tier5 (including 
open/test/hotspot/jtreg/containers/docker),  and tier6 tests passed .
        > 
        > [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8226575/webrev.02/ 
        > [2] Jira issue :https://bugs.openjdk.java.net/browse/JDK-8226575 
        > [3] CSR https://bugs.openjdk.java.net/browse/JDK-8228428 
        > 
        > Thank you,
        > -Daniil
        > 
        > 
        
        
    


Reply via email to