> On Jun 12, 2018, at 1:12 AM, David Holmes <david.hol...@oracle.com> wrote:
> 
> On 12/06/2018 9:30 AM, Bob Vandette wrote:
>>> On Jun 11, 2018, at 5:21 PM, David Holmes <david.hol...@oracle.com> wrote:
>>> 
>>> On 12/06/2018 12:12 AM, Bob Vandette wrote:
>>>>> On Jun 11, 2018, at 4:32 AM, David Holmes <david.hol...@oracle.com 
>>>>> <mailto:david.hol...@oracle.com>> wrote:
>>>>> 
>>>>> Sorry Bob I haven't had a chance to look at this detail.
>>>>> 
>>>>> For the Java code ... methods that return arrays should return 
>>>>> zero-length arrays when something is not available rather than null.
>>>> All methods do return zero length arrays except I missed the 
>>>> getPerCpuUsage.  I’ll fix that one and correct the javadoc.
>>> 
>>> There are a few more too:
>>> 
>> Those are covered by the function that converts the string range.
> 
> ??? I have no idea what you mean.
> 
> Java API design style is to return zero-length arrays rather than null. [Ref: 
> Effective Java First Edition, Item 27].

Look at line 174 in this file. 

http://cr.openjdk.java.net/~bobv/8203357/webrev.01/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java.html

Bob
> 
> Cheers,
> David
> -----
> 
>>> 231      * @return An array of available CPUs or null if metric is not 
>>> available.
>>> 232      *
>>> 233      */
>>> 234     public int[] getCpuSetCpus();
>>> 
>>> 242      * @return An array of available and online CPUs or null if the 
>>> metric
>>> 243      *         is not available.
>>> 244      *
>>> 245      */
>>> 246     public int[] getEffectiveCpuSetCpus();
>>> 
>>> 256      * @return An array of available memory nodes or null if metric is 
>>> not available.
>>> 257      *
>>> 258      */
>>> 259     public int[] getCpuSetMems();
>>> 
>>> 267      * @return An array of available and online nodes or null if the 
>>> metric
>>> 268      *         is not available.
>>> 269      *
>>> 270      */
>>> 271     public int[] getEffectiveCpuSetMems();
>>>>> 
>>>>> For getCpuPeriod() the term "operating system time slice" can be 
>>>>> misconstrued as being related to the scheduler timeslice that may, or may 
>>>>> not, exist, depending on the scheduler and scheduling policy etc. This 
>>>>> "timeslice" is something specific to cgroups - no?
>>>> The comments reads:
>>>>       * Returns the length of the operating system time slice, in
>>>>       * milliseconds, for processes within the Isolation Group.
>>>> The comment does infer that it’s process and cgroup (Isolation group) 
>>>> specific and not the generic os timeslice.
>>>> Isn’t this sufficient?
>>> 
>>> The phrase "operating system" makes this sound like some kind of global 
>>> timeslice notion - which it isn't. And I don't like to think of cpu 
>>> periods/shares/quotas in terms of "time slice" anyway. I don't see the 
>>> Docker or Cgroup documentation using "time slice" either. It suffices IMHO 
>>> to just say for period:
>>> 
>>> * Returns the length of the scheduling period, in
>>> * milliseconds, for processes within the Isolation Group.
>>> 
>>> then for quota:
>>> 
>>> * Returns the total available run-time allowed, in milliseconds,
>>> * during each scheduling period for all tasks in the Isolation Group.
>>> 
>> Ok. I’ll update the docs.
>> Bob
>>> Thanks,
>>> David
>>> 
>>>> Thanks,
>>>> Bob.
>>>>> 
>>>>> David
>>>>> 
>>>>>>> On 8/06/2018 3:43 AM, Bob Vandette wrote:
>>>>>>> Can I get one more reviewer for this RFE so I can integrate it?
>>>>>>> http://cr.openjdk.java.net/~bobv/8203357/webrev.01
>>>>>> Mandy Chung has reviewed this change.
>>>>>> I’ve run Mach5 hotspot and core lib tests.
>>>>>> I’ve reviewed the tests which were written by Harsha Wardhana
>>>>>> I filed a CSR for the command line change and it’s now approved and 
>>>>>> closed.
>>>>>> Thanks,
>>>>>> Bob.
>>>>>>> On May 30, 2018, at 3:45 PM, Bob Vandette <bob.vande...@oracle.com 
>>>>>>> <mailto:bob.vande...@oracle.com>> wrote:
>>>>>>> 
>>>>>>> Please review the following RFE which adds an internal API, along with 
>>>>>>> jtreg tests that provide
>>>>>>> access to Docker container configuration data and metrics.  In addition 
>>>>>>> to the API which we hope to
>>>>>>> take advantage of in the future with Java Flight Recorder and a JMX 
>>>>>>> Mbean, I’ve added an additional
>>>>>>> option to -XshowSettings:system than dumps out the container or host 
>>>>>>> cgroup confguration
>>>>>>> information.  See the sample output below:
>>>>>>> 
>>>>>>> RFE: Container Metrics
>>>>>>> 
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8203357
>>>>>>> 
>>>>>>> WEBREV:
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~bobv/8203357/webrev.01
>>>>>>> 
>>>>>>> 
>>>>>>> This commit will also include a fix for the following bug.
>>>>>>> 
>>>>>>> BUG: [TESTBUG] Test /runtime/containers/cgroup/PlainRead.java fails
>>>>>>> 
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8203691
>>>>>>> 
>>>>>>> WEBREV:
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~bobv/8203357/webrev.00/test/hotspot/jtreg/runtime/containers/cgroup/PlainRead.java.sdiff.html
>>>>>>> 
>>>>>>> SAMPLE USAGE and OUTPUT:
>>>>>>> 
>>>>>>> docker run —memory=256m --cpuset-cpus 4-7 -it ubuntu bash
>>>>>>> ./java -XshowSettings:system
>>>>>>> Operating System Metrics:
>>>>>>>    Provider: cgroupv1
>>>>>>>    Effective CPU Count: 4
>>>>>>>    CPU Period: 100000
>>>>>>>    CPU Quota: -1
>>>>>>>    CPU Shares: -1
>>>>>>>    List of Processors, 4 total:
>>>>>>>    4 5 6 7
>>>>>>>    List of Effective Processors, 4 total:
>>>>>>>    4 5 6 7
>>>>>>>    List of Memory Nodes, 2 total:
>>>>>>>    0 1
>>>>>>>    List of Available Memory Nodes, 2 total:
>>>>>>>    0 1
>>>>>>>    CPUSet Memory Pressure Enabled: false
>>>>>>>    Memory Limit: 256.00M
>>>>>>>    Memory Soft Limit: Unlimited
>>>>>>>    Memory & Swap Limit: 512.00M
>>>>>>>    Kernel Memory Limit: Unlimited
>>>>>>>    TCP Memory Limit: Unlimited
>>>>>>>    Out Of Memory Killer Enabled: true
>>>>>>> 
>>>>>>> TEST RESULTS:
>>>>>>> 
>>>>>>> testing runtime container APIs
>>>>>>> Directory "JTwork" not found: creating
>>>>>>> Passed: runtime/containers/cgroup/PlainRead.java
>>>>>>> Passed: runtime/containers/docker/DockerBasicTest.java
>>>>>>> Passed: runtime/containers/docker/TestCPUAwareness.java
>>>>>>> Passed: runtime/containers/docker/TestCPUSets.java
>>>>>>> Passed: runtime/containers/docker/TestMemoryAwareness.java
>>>>>>> Passed: runtime/containers/docker/TestMisc.java
>>>>>>> Test results: passed: 6
>>>>>>> Results written to /export/users/bobv/jdk11/build/jtreg/JTwork
>>>>>>> 
>>>>>>> testing jdk.internal.platform APIs
>>>>>>> Passed: jdk/internal/platform/cgroup/TestCgroupMetrics.java
>>>>>>> Passed: jdk/internal/platform/docker/TestDockerCpuMetrics.java
>>>>>>> Passed: jdk/internal/platform/docker/TestDockerMemoryMetrics.java
>>>>>>> Passed: jdk/internal/platform/docker/TestSystemMetrics.java
>>>>>>> Test results: passed: 4
>>>>>>> Results written to /export/users/bobv/jdk11/build/jtreg/JTwork
>>>>>>> 
>>>>>>> testing -XshowSettings:system launcher option
>>>>>>> Passed: tools/launcher/Settings.java
>>>>>>> Test results: passed: 1
>>>>>>> 
>>>>>>> 
>>>>>>> Bob.
>>>>>>> 
>>>>>>> 

Reply via email to