> On Dec 4, 2019, at 8:32 AM, Bob Vandette <[email protected]> wrote:
>
>
>> On Dec 3, 2019, at 9:00 PM, Daniil Titov <[email protected]> wrote:
>>
>> 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.
>
> I don’t think this approach will work. Both the array returned and the
> sysconf(_SC_NPROCESSORS_CONF)
> report the containers cpuset value so they will be equal causing you to
> always fallback.
>
> You can try to use containerMetrics.getPerCpuUsage() instead of
> containerMetrics.getEffectiveCpuSetCpus().
> The length of the array returned is the number of host cpus. Maybe Severin
> can confirm if this true in cgroupv2 as
> well.
I just checked the webrev for the cgroupv2 implementation and getPerCpuUsage is
not supported.
I still think it’s worth implementing this optimization but it won’t be used on
cgroupv2 since the
array length (0) won’t be equal to _SC_NPROCESSORS_CONF.
Here’s the cgroupv2 implementation of this method.
64 @Override
65 public long[] getPerCpuUsage() {
66 // Not supported
67 return new long[0];
68 }
Bob.
>
> Bob.
>
>
>>
>> Thank you,
>> Daniil
>>
>> On 12/3/19, 1:30 PM, "Bob Vandette" <[email protected]> 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 <[email protected]> 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
>>>
>>>
>>
>>
>>
>>
>
