Re: RFR: 8253939: [TESTBUG] Increase coverage of the cgroups detection code [v4]

[Bob Vandette]

On Wed, 28 Oct 2020 13:49:53 GMT, Severin Gehwolf  wrote:

>> Test only change. With 
>> [JDK-8253435](https://bugs.openjdk.java.net/browse/JDK-8253435) a test has 
>> been added on the hotspot side, but nothing for the Java Metrics code. Same 
>> for [JDK-8252359](https://bugs.openjdk.java.net/browse/JDK-8252359).
>> 
>> When JDK-8217766 got fixed cgroup factories with the detection logic didn't 
>> exist so were harder to test. This patch adds tests for them too.
>> 
>> Thoughts?
>
> Severin Gehwolf has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains one additional 
> commit since the last revision:
> 
>   8253939: [TESTBUG] Increase coverage of the cgroups detection code

Looks good.

-

Marked as reviewed by bobv (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/609

Re: RFR: 8245543: Cgroups: Incorrect detection logic on some systems (still reproducible) [v6]

[Bob Vandette]

On Thu, 8 Oct 2020 13:38:59 GMT, Severin Gehwolf  wrote:

>> An issue similar to 
>> [JDK-8239559](https://bugs.openjdk.java.net/browse/JDK-8239559) has been 
>> discovered. On the
>> affected system, `/proc/self/mountinfo` contains a line such as this one:
>> 
>> 35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup 
>> systemd rw,name=systemd
>>  
>> 
>> Note that `/proc/cgroups` looks like this:
>> 
>> #subsys_name hierarchy   num_cgroups enabled
>> cpuset   0   1   1
>> cpu  0   1   1
>> cpuacct  0   1   1
>> blkio0   1   1
>> memory   0   1   1
>> devices  0   1   1
>> freezer  0   1   1
>> net_cls  0   1   1
>> perf_event   0   1   1
>> net_prio 0   1   1
>> hugetlb  0   1   1
>> pids 0   1   1
>> 
>> This is in fact a cgroups v1 system with systemd put into a separate 
>> namespace via FS type `cgroup`. That mountinfo
>> line confuses the cgroups version detection heuristic. It only looked 
>> whether or not there is **any** entry in
>> mountinfo of FS-type `cgroup` . That's wrong. A better heuristic would be 
>> looking at controllers we care about: `cpu`,
>> `cpuacct`, `memory`, `cpuset` for hotspot and some more for the Java side. 
>> The proposed fix on the hotspot side is to
>> set `any_cgroup_mounts_found` to true only if one of those controllers show 
>> up in mountinfo. Otherwise, we know it's
>> cgroup v2 due to the zero hierarchy ids. The fix on the Java side is 
>> similar.  For the Java side the proposal is also
>> to do the parsing of the cgroup files in the factory now (single pass of 
>> files). No longer in the version specific
>> objects. In order to not regress here, I've added more tests.
>> **Testing**
>> 
>>  - [x] Linux x86_64 container tests on cgroup v1 (fastdebug)
>>  - [x] Linux x86_64 container tests on cgroup v2 (fastdebug)
>>  - [x] Added regression test which fails prior the patch and passes after
>>  - [x] Submit testing
>
> Severin Gehwolf has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Return false in case of no pattern match

Marked as reviewed by bobv (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/485

Re: RFR: 8245543: Cgroups: Incorrect detection logic on some systems (still reproducible) [v5]

[Bob Vandette]

On Thu, 8 Oct 2020 13:36:20 GMT, Severin Gehwolf  wrote:

>> src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
>>  line 185:
>> 
>>> 183:  } else {
>>> 184:// fallback to old, pre JDK-8245543, behaviour
>>> 185:return line.contains("cgroup");
>> 
>> Why are you doing this fallback rather than returning false??
>
> The idea was to have have some basics working even though the pattern doesn't 
> match for some reason. I guess we can
> remove that as it's rather unlikely to not match.

The change looks good.

-

PR: https://git.openjdk.java.net/jdk/pull/485

Re: RFR: 8245543: Cgroups: Incorrect detection logic on some systems (still reproducible) [v5]

[Bob Vandette]

On Thu, 8 Oct 2020 12:51:54 GMT, Severin Gehwolf  wrote:

>> An issue similar to 
>> [JDK-8239559](https://bugs.openjdk.java.net/browse/JDK-8239559) has been 
>> discovered. On the
>> affected system, `/proc/self/mountinfo` contains a line such as this one:
>> 
>> 35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup 
>> systemd rw,name=systemd
>>  
>> 
>> Note that `/proc/cgroups` looks like this:
>> 
>> #subsys_name hierarchy   num_cgroups enabled
>> cpuset   0   1   1
>> cpu  0   1   1
>> cpuacct  0   1   1
>> blkio0   1   1
>> memory   0   1   1
>> devices  0   1   1
>> freezer  0   1   1
>> net_cls  0   1   1
>> perf_event   0   1   1
>> net_prio 0   1   1
>> hugetlb  0   1   1
>> pids 0   1   1
>> 
>> This is in fact a cgroups v1 system with systemd put into a separate 
>> namespace via FS type `cgroup`. That mountinfo
>> line confuses the cgroups version detection heuristic. It only looked 
>> whether or not there is **any** entry in
>> mountinfo of FS-type `cgroup` . That's wrong. A better heuristic would be 
>> looking at controllers we care about: `cpu`,
>> `cpuacct`, `memory`, `cpuset` for hotspot and some more for the Java side. 
>> The proposed fix on the hotspot side is to
>> set `any_cgroup_mounts_found` to true only if one of those controllers show 
>> up in mountinfo. Otherwise, we know it's
>> cgroup v2 due to the zero hierarchy ids. The fix on the Java side is 
>> similar.  For the Java side the proposal is also
>> to do the parsing of the cgroup files in the factory now (single pass of 
>> files). No longer in the version specific
>> objects. In order to not regress here, I've added more tests.
>> **Testing**
>> 
>>  - [x] Linux x86_64 container tests on cgroup v1 (fastdebug)
>>  - [x] Linux x86_64 container tests on cgroup v2 (fastdebug)
>>  - [x] Added regression test which fails prior the patch and passes after
>>  - [x] Submit testing
>
> Severin Gehwolf has refreshed the contents of this pull request, and previous 
> commits have been removed. The
> incremental views will show differences compared to the previous content of 
> the PR. The pull request contains one new
> commit since the last revision:
>   8245543: Cgroups: Incorrect detection logic on some systems (still 
> reproducible)

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java 
line 185:

> 183:  } else {
> 184:// fallback to old, pre JDK-8245543, behaviour
> 185:return line.contains("cgroup");

Why are you doing this fallback rather than returning false??

-

PR: https://git.openjdk.java.net/jdk/pull/485

Re: RFR: 8245543: Cgroups: Incorrect detection logic on some systems (still reproducible) [v3]

[Bob Vandette]

On Wed, 7 Oct 2020 08:06:58 GMT, Severin Gehwolf  wrote:

>> OK. I'll probably fold JDK-8254001 into this one then.
>
> @bobvandette Done in the latest version. Thoughts?

I'm a little uncomfortable making such a dramatic change just to fix this small 
isolated problem.   I think about all
the churn we've had on these files and the backports to come.  Could you just 
add a
CgroupSubsystemFactory::isValidSubsystem function which checks for cpuset, 
cpuacct, memory, etc instead of the major
change?

-

PR: https://git.openjdk.java.net/jdk/pull/485

Re: RFR: 8245543: Cgroups: Incorrect detection logic on some systems (still reproducible) [v3]

[Bob Vandette]

On Mon, 5 Oct 2020 10:16:49 GMT, Severin Gehwolf  wrote:

>> An issue similar to 
>> [JDK-8239559](https://bugs.openjdk.java.net/browse/JDK-8239559) has been 
>> discovered. On the
>> affected system, `/proc/self/mountinfo` contains a line such as this one:
>> 
>> 35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup 
>> systemd rw,name=systemd
>>  
>> 
>> Note that `/proc/cgroups` looks like this:
>> 
>> #subsys_name hierarchy   num_cgroups enabled
>> cpuset   0   1   1
>> cpu  0   1   1
>> cpuacct  0   1   1
>> blkio0   1   1
>> memory   0   1   1
>> devices  0   1   1
>> freezer  0   1   1
>> net_cls  0   1   1
>> perf_event   0   1   1
>> net_prio 0   1   1
>> hugetlb  0   1   1
>> pids 0   1   1
>> 
>> This is in fact a cgroups v1 system with systemd put into a separate 
>> namespace via FS type `cgroup`. That mountinfo
>> line confuses the cgroups version detection heuristic. It only looked 
>> whether or not there is **any** entry in
>> mountinfo of FS-type `cgroup` . That's wrong. A better heuristic would be 
>> looking at controllers we care about: `cpu`,
>> `cpuacct`, `memory`, `cpuset` for hotspot and some more for the Java side. 
>> The proposed fix on the hotspot side is to
>> set `any_cgroup_mounts_found` to true only if one of those controllers show 
>> up in mountinfo. Otherwise, we know it's
>> cgroup v2 due to the zero hierarchy ids.  For the Java side the proposal is 
>> to add some parsing for mountinfo lines and
>> look for `cgroup` FS-type lines which aren't also mounted purely for systemd.
>> **Testing**
>> 
>>  - [x] Linux x86_64 container tests on cgroup v1 (fastdebug)
>>  - [x] Linux x86_64 container tests on cgroup v2 (fastdebug)
>>  - [x] Added regression test which fails prior the patch and passes after
>>  - [x] Submit testing
>
> Severin Gehwolf has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix typo in comment

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java 
line 150:

> 148: // find anyway in that case.
> 149: try (Stream mntInfo = 
> CgroupUtil.readFilePrivileged(Paths.get(mountInfo))) {
> 150: boolean anyCgroupMounted = 
> mntInfo.anyMatch(CgroupSubsystemFactory::noSystemdCgroupLine);

I'd prefer a similar approach to the hotspot side where you do a positive check 
for controllers we are interested in
rather than a negative check for systemd.

-

PR: https://git.openjdk.java.net/jdk/pull/485

Re: RFR: 8253476: TestUseContainerSupport.java fails on some Linux kernels w/o swap limit capabilities

[Bob Vandette]

On Tue, 29 Sep 2020 19:57:14 GMT, Coleen Phillimore  wrote:

>> Please review this small change to remove "--memory 200m" option from 
>> TestUseContainerSupport.java.  This can cause
>> test failures on systems where swap accounting is disabled.
>
> Marked as reviewed by coleenp (Reviewer).

Setting a 200MB container Limit when swap accounting is not enabled in the 
Kernel will cause a SEGV depending on how
much RAM is available in the host.  When we test with UseContainerSupport 
disabled and a container limit imposed by
docker, the VM assumes it has all host memory available to it.  The VM 
ergonomics will then assume it has a lot of
memory and will potentially select a very large heap.  When the heap is 
initialized, it will exceed the containers
limit causing the SEGV.

-

PR: https://git.openjdk.java.net/jdk/pull/303

Re: RFR: 8253476: TestUseContainerSupport.java fails on some Linux kernels w/o swap limit capabilities

[Bob Vandette]

On Tue, 22 Sep 2020 15:52:43 GMT, Harold Seigel  wrote:

> Please review this small change to remove "--memory 200m" option from 
> TestUseContainerSupport.java.  This can cause
> test failures on systems where swap accounting is disabled.

Marked as reviewed by bobv (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/303

Re: RFR: 8253476: TestUseContainerSupport.java fails on some Linux kernels w/o…

[Bob Vandette]

On Tue, 22 Sep 2020 15:52:43 GMT, Harold Seigel  wrote:

> Please review this small change to remove "--memory 200m" option from 
> TestUseContainerSupport.java.  This can cause
> test failures on systems where swap accounting is disabled.

Marked as reviewed by bobv (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/303

Re: RFR(s): 8250627: Use -XX:+/-UseContainerSupport for enabling/disabling Java container metrics

[Bob Vandette]

Looks good to me.

Bob.


> On Jul 28, 2020, at 10:01 AM, Severin Gehwolf  wrote:
> 
> Hi David,
> 
> Thanks for the review.
> 
> On Tue, 2020-07-28 at 23:49 +1000, David Holmes wrote:
>> Hi Severin,
>> 
>> On 28/07/2020 11:23 pm, Severin Gehwolf wrote:
>>> Hi David,
>>> 
>>> On Tue, 2020-07-28 at 21:17 +1000, David Holmes wrote:
 Hi Severin,
 
 On 28/07/2020 6:27 pm, Severin Gehwolf wrote:
> Hi,
> 
> Please review this patch which makes the Java container metrics adhere
> to -XX:+/-UseContainerSupport so they can be disabled if heuristics
> turn out to be wrong. The approach taken is to use JNI and call into
> the JVM in order to determine the setting of UseContainerSupport before
> Metrics are being instantiated.
> 
> The intention is for this patch to be backported to older JDKs so as to
> provide a means to disable container metrics should things go wrong
> with backports of the likes of JDK-8226575.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8250627
> webrev: 
> https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8250627/01/webrev/
 
 Seems quite simple and clean.
 
 One query though, I'm not clear on who the expected caller of
 Metrics.getInstance() is? (Coming from the perspective of "might we want
 to cache the fact container support is not enabled?".)
>>> 
>>> I know of two uses so far:
>>> 
>>> 1) Launcher (-XshowSettings:system):
>>> http://hg.openjdk.java.net/jdk/jdk/file/89fe9e02a522/src/java.base/share/classes/sun/launcher/LauncherHelper.java#l318
>>> 
>>> 2) OperatingSystemMXBean:
>>> http://hg.openjdk.java.net/jdk/jdk/file/89fe9e02a522/src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java#l48
>>> 
>>> Both uses seem OK as is. Is it worth caching something here?
>> 
>> No that looks fine. I didn't find them because of the reflective 
>> invocation in Metrics.systemMetrics().
>> 
 Also note that we no longer update JVM_INTERFACE_VERSION (last update
 was JDK 13) - it is meaningless now the JDK and hotspot are in sync
 version wise.
>>> 
>>> OK. Does that mean I should revert the version increment, then?
>> 
>> I would leave it unchanged, yes.
> 
> Here you go:
> https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8250627/02/webrev/
> 
> OK?
> 
> Thanks,
> Severin
> 

Re: No way to disable Java Container Metrics

[Bob Vandette]

> On Jul 27, 2020, at 11:32 AM, Severin Gehwolf  wrote:
> 
> Hi Bob,
> 
> On Fri, 2020-07-24 at 15:21 -0400, Bob Vandette wrote:
>> I’ve been monitoring the discussion on your jdk8u alias and I can’t help 
>> wondering if
>> my original decision to provide two different implementations of this 
>> container detection
>> logic was the best way to go.
>> 
>> I didn’t want to have an all Java implementation since the VM needs to 
>> initialize it’s
>> memory and cpu sizing very early on prior to its ability to run Java code.
>> 
>> If we put all of the logic in the VM, then we’d end up with a pretty wide 
>> interface to
>> the VM and more overhead extracting values (due to JNI) although the Java 
>> logic 
>> typically ends up in native code anyway.  Having the functionality in the VM
>> makes it easier to add JFR events.  Having a pure Java implementation makes 
>> it
>> easier to support the OS MBeans.  The maintenance of supporting both 
>> implementations
>> is a bit of a pain.
> 
> Add to that that Java metrics return non-null for any controller it
> finds. The JVM doesn't. Container support is turned on there only if
> all cpu and memory controllers are found.

That was intentional.  I wanted the VM to configure itself consistently.
Either all needed value are present or revert to non container mode.
For Metrics, I felt it was ok to report whatever is available.

Bob.


> 
>> Assuming no one has the time or desire to migrate the logic to the VM and 
>> provide
>> Java wrapper logic, I’m ok with what you are proposing.  It’s one step on 
>> the path.
>> The VM support and the Java level support are really for two different 
>> consumers
>> but I think it would be cleaner and less confusing to disable both on one 
>> flag rather
>> than support two options.
> 
> OK, agreed. I've filed:
> https://bugs.openjdk.java.net/browse/JDK-8250627
> 
> Thanks,
> Severin
> 
>> Bob.
>> 
>>> On Jul 24, 2020, at 12:13 PM, Severin Gehwolf 
>>> wrote:
>>> 
>>> Hi,
>>> 
>>> For hotspot one can disable container detection with a simple
>>> switch:
>>> 
>>> $ java -XX:-UseContainerSupport -Xlog:os+container=trace -version
>>> [0.000s][trace][os,container] OSContainer::init: Initializing
>>> Container Support
>>> [0.000s][trace][os,container] Container Support not enabled
>>> openjdk version "15-internal" 2020-09-15
>>> OpenJDK Runtime Environment (build 15-internal+0-
>>> adhoc.sgehwolf.openjdk-head-2)
>>> OpenJDK 64-Bit Server VM (build 15-internal+0-
>>> adhoc.sgehwolf.openjdk-head-2, mixed mode, sharing)
>>> 
>>> The same cannot be achieved with the Java code,
>>> jdk.internal.platform.Metrics.java and friends in the JDK. At the
>>> time
>>> Metrics were added the only consumer of them was the Java Launcher
>>> via
>>> -XshowSettings:system. This has changed with JDK-8226575:
>>> OperatingSystemMXBean should be made container aware.
>>> 
>>> It is known that in certain cases the container detection code will
>>> detect for a system to be supposedly in a container where it
>>> actually
>>> isn't:
>>> https://bugs.openjdk.java.net/browse/JDK-8227006?focusedCommentId=14275604=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14275604
>>> 
>>> For hotspot there is a flag, to turn auto-detection off for exactly
>>> the
>>> case when heuristics are wrong: -XX:-UseContainerSupport. However,
>>> this
>>> flag has no effect on the Java metrics code. There is a risk that
>>> on
>>> some systems the auto-detection will be wrong and might cause
>>> regressions.
>>> 
>>> I propose to make the Java metrics code adhere to -XX:+/-
>>> UseContainerSupport flag. Do you think this would be useful? If
>>> yes,
>>> I'll file a bug and propose a patch for it.
>>> 
>>> Thoughts? Opinions?
>>> 
>>> Thanks,
>>> Severin
>>> 
> 

Re: No way to disable Java Container Metrics

[Bob Vandette]

I’ve been monitoring the discussion on your jdk8u alias and I can’t help 
wondering if
my original decision to provide two different implementations of this container 
detection
logic was the best way to go.

I didn’t want to have an all Java implementation since the VM needs to 
initialize it’s
memory and cpu sizing very early on prior to its ability to run Java code.

If we put all of the logic in the VM, then we’d end up with a pretty wide 
interface to
the VM and more overhead extracting values (due to JNI) although the Java logic 
typically ends up in native code anyway.  Having the functionality in the VM
makes it easier to add JFR events.  Having a pure Java implementation makes it
easier to support the OS MBeans.  The maintenance of supporting both 
implementations
is a bit of a pain.

Assuming no one has the time or desire to migrate the logic to the VM and 
provide
Java wrapper logic, I’m ok with what you are proposing.  It’s one step on the 
path.
The VM support and the Java level support are really for two different consumers
but I think it would be cleaner and less confusing to disable both on one flag 
rather
than support two options.

Bob.

> On Jul 24, 2020, at 12:13 PM, Severin Gehwolf  wrote:
> 
> Hi,
> 
> For hotspot one can disable container detection with a simple switch:
> 
> $ java -XX:-UseContainerSupport -Xlog:os+container=trace -version
> [0.000s][trace][os,container] OSContainer::init: Initializing Container 
> Support
> [0.000s][trace][os,container] Container Support not enabled
> openjdk version "15-internal" 2020-09-15
> OpenJDK Runtime Environment (build 
> 15-internal+0-adhoc.sgehwolf.openjdk-head-2)
> OpenJDK 64-Bit Server VM (build 15-internal+0-adhoc.sgehwolf.openjdk-head-2, 
> mixed mode, sharing)
> 
> The same cannot be achieved with the Java code,
> jdk.internal.platform.Metrics.java and friends in the JDK. At the time
> Metrics were added the only consumer of them was the Java Launcher via
> -XshowSettings:system. This has changed with JDK-8226575:
> OperatingSystemMXBean should be made container aware.
> 
> It is known that in certain cases the container detection code will
> detect for a system to be supposedly in a container where it actually
> isn't:
> https://bugs.openjdk.java.net/browse/JDK-8227006?focusedCommentId=14275604=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14275604
> 
> For hotspot there is a flag, to turn auto-detection off for exactly the
> case when heuristics are wrong: -XX:-UseContainerSupport. However, this
> flag has no effect on the Java metrics code. There is a risk that on
> some systems the auto-detection will be wrong and might cause
> regressions.
> 
> I propose to make the Java metrics code adhere to -XX:+/-
> UseContainerSupport flag. Do you think this would be useful? If yes,
> I'll file a bug and propose a patch for it.
> 
> Thoughts? Opinions?
> 
> Thanks,
> Severin
> 

Re: [PING] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy

[Bob Vandette]

> On Feb 18, 2020, at 2:00 PM, Mandy Chung  wrote:
> 
> 
> 
> On 2/18/20 4:50 AM, Severin Gehwolf wrote:
>> Hi Mandy,
>> 
>> Thanks again for the review!
>> 
>> Updated webrev:
>> incremental (only review changes): 
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/11/incremental/webrev/
>> 
>> full: 
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/11/webrev/
> 
> This looks good.  I only skimmed on the tests and not reviewed in details (I 
> assume Bob has reviewed them).   

Yes, I checked the tests and they look fine.

Bob.

> 
>  All new cgroup-specific and metrics implementation classes are now 
> linux-specific classes which is good.
> 
>> More below.
>> 
>> 
>> 
>>> test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
>>>The Oracle copyright is taken out and the copyright is also changed from 
>>> GPL to GPL+CP.
>>>The Red Hat copyright can be added to the top of the file immediately 
>>> before "DO NOT ALTER or REMOVE" line like [1].
>>> 
>>> [1] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/tip/test/hotspot/jtreg/compiler/onSpinWait/TestOnSpinWaitEnableDisable.java
>> Hmm, old MetricsTester got renamed with this patch to
>> MetricsTesterCgroupV1. MetricsTesterCgroupV1 still has the old
>> copyright. The version you've looked at is the common part and
>> instantiates tester for cgroup v1 or cgroup v2 as required.
>> 
> 
> Thanks for clarifying.  I now see that MetricsTester.java is a new file in 
> this patch but the webrev shows as an existing file.  
> 
>> Aside: Not sure why old MetricsTester (or new MetricsTesterCgroupV1) is
>> GPL (over GPL+CP).
>> Either way, I've changed license to GPL over GPL+CP for the new test
>> classes with Red Hat copyright.
>> 
> 
> I skimmed through the copyright header and license text.  Looks fine to me.
> 
> Mandy
> 

Re: [PING] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy

[Bob Vandette]

I applied the delegation change that you recommended and now all container 
tests pass.

Here’s the change that I applied.

echo +cpu +cpuset +io +memory +pids > /sys/fs/cgroup/cgroup.subtree_control
echo +cpu +cpuset +io +memory +pids > 
/sys/fs/cgroup/user.slice/cgroup.subtree_control
echo +cpu +cpuset +io +memory +pids > 
/sys/fs/cgroup/user.slice/user-23603.slice/cgroup.subtree_control

Although all the tests now pass, the values for cpusets and cpusets.mems still 
do not show up on the host
or container unless limits are specified.  That’s the only remaining minor 
difference from cgroupv1 that I can see.

testing container APIs
Directory "JTwork" not found: creating
Passed: containers/cgroup/PlainRead.java
Passed: containers/docker/DockerBasicTest.java
Passed: containers/docker/TestCPUAwareness.java
Passed: containers/docker/TestCPUSets.java
Passed: containers/docker/TestJcmdWithSideCar.java
Passed: containers/docker/TestJFREvents.java
Passed: containers/docker/TestJFRNetworkEvents.java
Passed: containers/docker/TestMemoryAwareness.java
Passed: containers/docker/TestMisc.java
Test results: passed: 9
Results written to /export/users/bobv/jdk15/build/jtreg/JTwork
testing jdk.internal.platform APIs
Passed: jdk/internal/platform/cgroup/TestCgroupMetrics.java
Passed: jdk/internal/platform/cgroup/TestCgroupSubsystemController.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: 5
Results written to /export/users/bobv/jdk15/build/jtreg/JTwork
testing -XshowSettings:system launcher option
Passed: tools/launcher/Settings.java
Test results: passed: 1
Results written to /export/users/bobv/jdk15/build/jtreg/JTwork

Bob.

> On Feb 12, 2020, at 10:13 AM, Bob Vandette  wrote:
> 
> 
>> On Feb 12, 2020, at 6:22 AM, Severin Gehwolf  wrote:
>> 
>> Hi Bob,
>> 
>> On Tue, 2020-02-11 at 16:59 -0500, Bob Vandette wrote:
>>> I applied your patch to the latest JDK 15 sources and ran the container
>>> tests on Oracle Linux 8.1 with podman/cgroupv2 enabled.  There were some 
>>> issues.
>>> I’m not sure if its my setup or not.  
>> 
>> Looking over them, they appear to be setup issues.
>> 
>> Getting the setup working right was tricky for me as well.
>> 
>> Aside:
>> podman has a --runtime switch which you could point to a cgroups v2
>> capable crun binary for example.
>> https://bugs.openjdk.java.net/browse/JDK-8230305 has some examples of
>> how to use it.
>> 
>> I also needed some extra setup to get the controllers' delegation to
>> work:
>> https://urldefense.com/v3/__https://scrivano.org/2019/02/26/resources-management-with-rootless-containers/__;!!GqivPVa7Brio!Ot7tv-QJgGntRyaimzIRrRh7rqvnzMu9AoOaBWINWNnnpR9LUVNNvGUe9GXp0Ri0Wg$
>>  
>> 
>>> I also ran the same build on Ubuntu with docker/cgroupv1.  I didn't see any 
>>> failures on
>>> the cgroupv1 system.
>> 
>> Great, thanks!
>> 
>>> Here are some notes:
>>> 
>>> The podman version on OL 8.1 doesn't yes support cgroupv2.  I
>>> built the latest from sources for this test.
>>> 
>>> The docker tests take a very long time under podman!  Longer than
>>> the cgroupv1 run.
>> 
>> Possibly. I haven't done any fair comparison of the two.
> 
> It’s possible that it’s trying to access several container hubs.  I may be 
> getting some timeouts
> since I haven’t figured out how to specify proxies for podman.  I’ve set a 
> local http_proxy env variable
> but maybe that’s not working.
> 
>> 
>>> cpusets and cpusets.mems are blank on host and if none are specified
>>> on podman/docker run command.  On cgroupv1 they are host values if not
>>> specified for container.  Effective cpusets and cpusets.mems are set 
>>> properly
>>> in a container.
>> 
>> Yes, there seems to be slight differences to cgroup v1. You can only
>> set cpusets on containers if the host has the controller enabled,
>> though:
>> 
>> # cat /sys/fs/cgroup/cgroup.subtree_control
>> cpuset cpu io memory pids
>> # podman run --memory=300M --cpuset-cpus=0,1 -ti -v `pwd`:/mnt fedora:30 
>> /bin/bash
>> [root@5d4a4e593a24 /]# cat /sys/fs/cgroup$(cat /proc/self/cgroup | cut -d':' 
>> -f3)/cpuset.cpus
>> 0-1
>> [root@5d4a4e593a24 mnt]# ./cgroupsv2-jdk/bin/java -XshowSettings:system 
>> -version
>> Operating System Metrics:
>>   Provider: cgroupv2
>>   Effective CPU Count: 2
>>   CPU Period: 10us
>>   CPU Quota: -1
>>   CPU Shares: -1
>>   List of Processors, 2 

Re: [PING] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy

[Bob Vandette]

> On Feb 12, 2020, at 6:22 AM, Severin Gehwolf  wrote:
> 
> Hi Bob,
> 
> On Tue, 2020-02-11 at 16:59 -0500, Bob Vandette wrote:
>> I applied your patch to the latest JDK 15 sources and ran the container
>> tests on Oracle Linux 8.1 with podman/cgroupv2 enabled.  There were some 
>> issues.
>> I’m not sure if its my setup or not.  
> 
> Looking over them, they appear to be setup issues.
> 
> Getting the setup working right was tricky for me as well.
> 
> Aside:
> podman has a --runtime switch which you could point to a cgroups v2
> capable crun binary for example.
> https://bugs.openjdk.java.net/browse/JDK-8230305 has some examples of
> how to use it.
> 
> I also needed some extra setup to get the controllers' delegation to
> work:
> https://urldefense.com/v3/__https://scrivano.org/2019/02/26/resources-management-with-rootless-containers/__;!!GqivPVa7Brio!Ot7tv-QJgGntRyaimzIRrRh7rqvnzMu9AoOaBWINWNnnpR9LUVNNvGUe9GXp0Ri0Wg$
>  
> 
>> I also ran the same build on Ubuntu with docker/cgroupv1.  I didn't see any 
>> failures on
>> the cgroupv1 system.
> 
> Great, thanks!
> 
>> Here are some notes:
>> 
>> The podman version on OL 8.1 doesn't yes support cgroupv2.  I
>> built the latest from sources for this test.
>> 
>> The docker tests take a very long time under podman!  Longer than
>> the cgroupv1 run.
> 
> Possibly. I haven't done any fair comparison of the two.

It’s possible that it’s trying to access several container hubs.  I may be 
getting some timeouts
since I haven’t figured out how to specify proxies for podman.  I’ve set a 
local http_proxy env variable
but maybe that’s not working.

> 
>> cpusets and cpusets.mems are blank on host and if none are specified
>> on podman/docker run command.  On cgroupv1 they are host values if not
>> specified for container.  Effective cpusets and cpusets.mems are set properly
>> in a container.
> 
> Yes, there seems to be slight differences to cgroup v1. You can only
> set cpusets on containers if the host has the controller enabled,
> though:
> 
> # cat /sys/fs/cgroup/cgroup.subtree_control
> cpuset cpu io memory pids
> # podman run --memory=300M --cpuset-cpus=0,1 -ti -v `pwd`:/mnt fedora:30 
> /bin/bash
> [root@5d4a4e593a24 /]# cat /sys/fs/cgroup$(cat /proc/self/cgroup | cut -d':' 
> -f3)/cpuset.cpus
> 0-1
> [root@5d4a4e593a24 mnt]# ./cgroupsv2-jdk/bin/java -XshowSettings:system 
> -version
> Operating System Metrics:
>Provider: cgroupv2
>Effective CPU Count: 2
>CPU Period: 10us
>CPU Quota: -1
>CPU Shares: -1
>List of Processors, 2 total: 
>0 1 
>List of Effective Processors, 2 total: 
>0 1 
>List of Memory Nodes: N/A
>List of Available Memory Nodes, 1 total: 
>0 
>Memory Limit: 300.00M
>Memory Soft Limit: Unlimited
>Memory & Swap Limit: 600.00M
> 
> openjdk version "15-internal" 2020-09-15
> OpenJDK Runtime Environment (build 
> 15-internal+0-adhoc.sgehwolf.openjdk-head-2)
> OpenJDK 64-Bit Server VM (build 15-internal+0-adhoc.sgehwolf.openjdk-head-2, 
> mixed mode, sharing)
> 

In my setup, your command above works correctly.  The only thing that doesn’t 
work is if I run the -XshowSettings
directly on the host OR I run a container without specifying —cpuset-cpus.  The 
default setup seems wrong.

> 
>> HOST OUTPUT:
>> ./java -XshowSettings:system -version
>> Operating System Metrics:
>>Provider: cgroupv2
>>Effective CPU Count: 32
>>CPU Period: -1
>>CPU Quota: -1
>>CPU Shares: -1
>>List of Processors: N/A
>>List of Effective Processors: N/A
>>List of Memory Nodes: N/A
>>List of Available Memory Nodes: N/A
>>Memory Limit: Unlimited
>>Memory Soft Limit: Unlimited
>>Memory & Swap Limit: Unlimited
>> 
>> CONTAINER OUTPUT:
>> # podman run -it -v `pwd`:/mnt ubuntu bash
>> root@3c6654a3b834:/mnt/jdk/bin# ./java -XshowSettings:system -version
>> Operating System Metrics:
>>Provider: cgroupv2
>>Effective CPU Count: 32
>>CPU Period: 10us
>>CPU Quota: -1
>>CPU Shares: -1
>>List of Processors: N/A
>>List of Effective Processors, 32 total: 
>>0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 
>> 28 29 30 31 
>>List of Memory Nodes: N/A
>>List of Available Memory Nodes, 1 total: 
>>0 
>>Memory Limit: Unlimited
>>Memory Soft Limit: Unlimited
>>Memory & Swap Limit: Unlimited
> 
> Yes, host and container output differ depending on the configur

Re: [PING] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy

[Bob Vandette]

I applied your patch to the latest JDK 15 sources and ran the container
tests on Oracle Linux 8.1 with podman/cgroupv2 enabled.  There were some issues.
I’m not sure if its my setup or not.  

I also ran the same build on Ubuntu with docker/cgroupv1.  I didn't see any 
failures on
the cgroupv1 system.

Here are some notes:

The podman version on OL 8.1 doesn't yes support cgroupv2.  I
built the latest from sources for this test.

The docker tests take a very long time under podman!  Longer than
the cgroupv1 run.

cpusets and cpusets.mems are blank on host and if none are specified
on podman/docker run command.  On cgroupv1 they are host values if not
specified for container.  Effective cpusets and cpusets.mems are set properly
in a container.

HOST OUTPUT:
./java -XshowSettings:system -version
Operating System Metrics:
Provider: cgroupv2
Effective CPU Count: 32
CPU Period: -1
CPU Quota: -1
CPU Shares: -1
List of Processors: N/A
List of Effective Processors: N/A
List of Memory Nodes: N/A
List of Available Memory Nodes: N/A
Memory Limit: Unlimited
Memory Soft Limit: Unlimited
Memory & Swap Limit: Unlimited

CONTAINER OUTPUT:
# podman run -it -v `pwd`:/mnt ubuntu bash
root@3c6654a3b834:/mnt/jdk/bin# ./java -XshowSettings:system -version
Operating System Metrics:
Provider: cgroupv2
Effective CPU Count: 32
CPU Period: 10us
CPU Quota: -1
CPU Shares: -1
List of Processors: N/A
List of Effective Processors, 32 total: 
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 
28 29 30 31 
List of Memory Nodes: N/A
List of Available Memory Nodes, 1 total: 
0 
Memory Limit: Unlimited
Memory Soft Limit: Unlimited
Memory & Swap Limit: Unlimited


Docker tests fail if /bin/docker is not available in podman setup.  We
probably should enhance the docker check to also look for podman.

Two container tests failed:

FAILED: containers/cgroup/PlainRead.java failed Memory Limit is: -2 instead
of unlimited or -1.  This is because memory.max is not foumd.

FAILED: jdk/internal/platform/cgroup/TestCgroupMetrics.java
This fails because nr_periods line doesn't always exist.  I think you’ve got
to enable a quota for this to appear (not sure).  

Here’s the contents:
% more cpu.stat
usage_usec 23974562755
user_usec 22257183568
system_usec 1717379186

CGROUPV2 results on Oracle Linux 8.1
-

testing container APIs

FAILED: containers/cgroup/PlainRead.java
Passed: containers/docker/DockerBasicTest.java
Passed: containers/docker/TestCPUAwareness.java
Passed: containers/docker/TestCPUSets.java
Passed: containers/docker/TestJcmdWithSideCar.java
Passed: containers/docker/TestJFREvents.java
Passed: containers/docker/TestJFRNetworkEvents.java
Passed: containers/docker/TestMemoryAwareness.java
Passed: containers/docker/TestMisc.java
Test results: passed: 8; failed: 1
Results written to /export/users/bobv/jdk15/build/jtreg/JTwork
Error: Some tests failed or other problems occurred.

testing jdk.internal.platform APIs

FAILED: jdk/internal/platform/cgroup/TestCgroupMetrics.java
Passed: jdk/internal/platform/cgroup/TestCgroupSubsystemController.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; failed: 1
Results written to /export/users/bobv/jdk15/build/jtreg/JTwork
Error: Some tests failed or other problems occurred.

testing -XshowSettings:system launcher option

Passed: tools/launcher/Settings.java
Test results: passed: 1

Bob.

> On Feb 11, 2020, at 1:04 PM, Severin Gehwolf  wrote:
> 
> Hi Mandy, Bob,
> 
> Thanks again for the reviews and patience on this. Sorry it took me so
> long to get back to this :-/
> 
> Updated webrev:
> Full: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/10/webrev/
> incremental: 
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/10/incremental/webrev/
> 
> I've tested this with docker tests on cgroup v1 and via podman on a
> cgroup v2 system. They pass. I'll be running this through jdk-submit as
> well.
> 
> More below.
> 
> On Tue, 2020-01-21 at 16:09 -0800, Mandy Chung wrote:
>> Hi Severin,
>> 
>> Thanks for the update.
>> 
>> On 1/21/20 11:30 AM, Severin Gehwolf wrote:
>>> Full: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/09/webrev/
>>> incremental: 
>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-823/09/incremental/webrev/
>>> 
>> 
>> I have answered my own question.   Most of the metrics used to return 0 if 
>> unavailable due to IOException reading a file or malformed content and now 
>> are changed to return -2 due to error fetching the metric.
>> 
>> The following are about limits which used to return -1 if unlimited or no 
>> limit set.
>>public long getCpuQuota();
>>public long getCpuShares();
>>public long getMemoryLimit();
>>public long 

Re: [PING] RFR: 8231111: Cgroups v2: Rework Metrics in java.base so as to recognize unified hierarchy

[Bob Vandette]

> On Jan 22, 2020, at 11:06 AM, Mandy Chung  wrote:
> 
> 
> 
> On 1/22/20 1:53 AM, Severin Gehwolf wrote:
>> Hi Mandy,
>> 
>> Thanks again for the review! I'll be sure to fix things. Some of the
>> issues you've pointed out probably pre-existed in old code. Some became
>> more complicated since unlimited in cgroupv1 is a large long value in
>> files. Anyway, I'll update it.
> 
> Thank you.
>> On Tue, 2020-01-21 at 16:09 -0800, Mandy Chung wrote:
>>> MetricsCgroupV1 is linux-only.  It will fail the compilation when
>>> building on non-linux.
>> I don't think so. MetricsCgroupV1 is a new interface in shared code
>> just like Metrics itself:
>>  src/java.base/share/classes/jdk/internal/platform/MetricsCgroupV1.java
> 
> Ah, I mis-read that was placed under src/java.base/linux/classes. OK.
>> I've put it there for the exact reason to NOT break compilation on non-
>> Linux. We could call it ExtendedMetrics or something, though. Pondered
>> that for a bit, but it wasn't important enough so I've kept it.
>>> One option is to move this code to
>>>src/java.base/linux/share/sun/launcher/CgroupMetrics.java
>> Not sure if that would help. MetricsCgropuV1 is being used in
>> LauncherHelper.java which itself is available on all platforms, no?
> 
> -XshowSettings:system is a linux-only option.
>>> Are they continued to be interesting metrics to be output from
>>> -XshowSetting?  I wonder if they can simply be dropped from the output.
>>> Bob will have an opinion.
>> I think they are. At least it prints the same metrics as were printed
>> before this patch on cgroupv1. What's the rationale of not printing
>> them any longer? My premise was, they were useful before and they
>> continue to be useful. For cgroupv2 they're not configurable so they're
>> not printed (and not useful as they'd show up as N/A anyway).
> 
> Bob can advice on the usefulness of these metrics.I have no objection to 
> print them on cgroup v1.  On v2,  they are not shown (not even N/A, right?)

These metrics are not that useful in the -XshowSettings context.  I’d just drop 
them from the output.

Bob.


> 
> Mandy

Re: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

   36 log(String.format("OperatingSystemMXBean.getTotalMemorySize: %d", 
>osBean.getTotalMemorySize()));
>   37 
>log(String.format("OperatingSystemMXBean.getTotalPhysicalMemorySize: 
>%d", osBean.getTotalPhysicalMemorySize()));
>   38 log(String.format("OperatingSystemMXBean.getFreeMemorySize: %d", 
>osBean.getFreeMemorySize()));
>   39 
>log(String.format("OperatingSystemMXBean.getFreePhysicalMemorySize: %d", 
>osBean.getFreePhysicalMemorySize()));
>   40 log(String.format("OperatingSystemMXBean.getTotalSwapSpaceSize: 
>%d", osBean.getTotalSwapSpaceSize()));
>   41 log(String.format("OperatingSystemMXBean.getFreeSwapSpaceSize: 
>%d", osBean.getFreeSwapSpaceSize()));
>   42 log(String.format("OperatingSystemMXBean.getCpuLoad: %f", 
>osBean.getCpuLoad()));
>   43 log(String.format("OperatingSystemMXBean.getSystemCpuLoad: %f", 
>osBean.getSystemCpuLoad()));
> 
> 
>Thanks,
>Serguei
> 
> 
> 
>On 12/6/19 17:41, Daniil Titov wrote:
>> Hi David, Mandy, and Bob,
>> 
>> Thank you for reviewing this fix.
>> 
>> Please review a new version of the fix [1] that includes the following 
>> changes comparing to the previous version of the webrev ( webrev.04)
>> 1. The changes in Javadoc made in the webrev.04 comparing to webrev.03 and 
>> to CSR [3] were discarded.
>> 2.  The implementation of methods getFreeMemorySize, getTotalMemorySize, 
>> getFreeSwapSpaceSize and getTotalSwapSpaceSize
>>  was also reverted to webrev.03 version that return host's values if the 
>> metrics are unavailable or cannot be properly read.
>>  I would like to mention that  currently the native implementation of 
>> these methods de-facto may return -1 at some circumstances,
>>  but I agree that the changes proposed in the previous version of the 
>> webrev increase such probability.
>>  I filed the follow-up issue [4] as Mandy suggested.
>> 3.  The legacy methods were renamed as David suggested.
>> 
>> 
>>> src/jdk.management/linux/native/libmanagement_ext/UnixOperatingSystem.c
>>> ! static int initialized=1;
>>> 
>>>  Am I reading this right that the code currently fails to actually do the
>>> initialization because of this ???
>> Yes, currently the code fails to do the initialization but it was unnoticed 
>> since method
>> get_cpuload_internal(...) was never called for a specific CPU, the first 
>> parameter "which"
>> was always -1.
>> 
>>>  test/hotspot/jtreg/containers/docker/CheckOperatingSystemMXBean.java
>>> 
>>> System.out.println(String.format(...)
>>> 
>>> Why not simply
>>> 
>>> System.out.printf(..)
>> As I tried explain it earlier it would make the tests unstable.
>> System.out.printf(...) just delegates the call to System.out.format(...) 
>> that doesn't emit the string atomically.
>> Instead it parses the format string into a list of FormatString objects and 
>> then iterates over the list.
>> As a result, the other traces occasionally got printed between these 
>> iterations  and break the pattern the test is expected to find
>> in the output.
>> 
>> For example, here is the sample of a such output that has the trace message 
>> printed between " OperatingSystemMXBean.getFreePhysicalMemorySize:"
>> and "1030762496".
>> 
>> 
>> [0.304s][trace][os,container] Memory Usage is: 42983424
>> OperatingSystemMXBean.getFreeMemorySize: 1030758400
>> [0.305s][trace][os,container] Path to /memory.usage_in_bytes is 
>> /sys/fs/cgroup/memory/memory.usage_in_bytes
>> [0.305s][trace][os,container] Memory Usage is: 42979328
>> [0.306s][trace][os,container] Path to /memory.usage_in_bytes is 
>> /sys/fs/cgroup/memory/memory.usage_in_bytes
>> OperatingSystemMXBean.getFreePhysicalMemorySize: 
>> [0.306s][trace][os,container] Memory Usage is: 42975232
>> 1030762496
>> OperatingSystemMXBean.getTotalSwapSpaceSize: 499122176
>> 
>> 
>> java.lang.RuntimeException: 
>> 'OperatingSystemMXBean\\.getFreePhysicalMemorySize: [1-9][0-9]+' missing 
>> from stdout/stderr
>> 
>>  at 
>> jdk.test.lib.process.OutputAnalyzer.shouldMatch(OutputAnalyzer.java:306)
>>  at 
>> TestMemoryAwareness.testOperatingSystemMXBeanAwareness(TestMemoryAwareness.java:151)
>>  at TestMemoryAwareness.main(TestMemoryAwareness.java:73)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invok

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

Why did you not change the exception caught in SubSystem.java:getStringValue to 
PrivilegedActionException from IOException
so it’s consistent with the other get functions?

Bob.


> On Dec 6, 2019, at 8:41 PM, Daniil Titov  wrote:
> 
> Hi David, Mandy, and Bob,
> 
> Thank you for reviewing this fix.
> 
> Please review a new version of the fix [1] that includes the following 
> changes comparing to the previous version of the webrev ( webrev.04)
> 1. The changes in Javadoc made in the webrev.04 comparing to webrev.03 and to 
> CSR [3] were discarded.
> 2.  The implementation of methods getFreeMemorySize, getTotalMemorySize, 
> getFreeSwapSpaceSize and getTotalSwapSpaceSize
> was also reverted to webrev.03 version that return host's values if the 
> metrics are unavailable or cannot be properly read.
> I would like to mention that  currently the native implementation of 
> these methods de-facto may return -1 at some circumstances,
> but I agree that the changes proposed in the previous version of the 
> webrev increase such probability.
> I filed the follow-up issue [4] as Mandy suggested.
> 3.  The legacy methods were renamed as David suggested.
> 
> 
>> src/jdk.management/linux/native/libmanagement_ext/UnixOperatingSystem.c
>> ! static int initialized=1;
>> 
>> Am I reading this right that the code currently fails to actually do the
>> initialization because of this ???
> 
> Yes, currently the code fails to do the initialization but it was unnoticed 
> since method 
> get_cpuload_internal(...) was never called for a specific CPU, the first 
> parameter "which"
> was always -1.
> 
>> test/hotspot/jtreg/containers/docker/CheckOperatingSystemMXBean.java
>> 
>> System.out.println(String.format(...)
>> 
>> Why not simply
>> 
>> System.out.printf(..)
> 
> As I tried explain it earlier it would make the tests unstable.
> System.out.printf(...) just delegates the call to System.out.format(...) that 
> doesn't emit the string atomically.
> Instead it parses the format string into a list of FormatString objects and 
> then iterates over the list.
> As a result, the other traces occasionally got printed between these 
> iterations  and break the pattern the test is expected to find
> in the output.
> 
> For example, here is the sample of a such output that has the trace message 
> printed between " OperatingSystemMXBean.getFreePhysicalMemorySize:"
> and "1030762496".
> 
> 
> [0.304s][trace][os,container] Memory Usage is: 42983424
> OperatingSystemMXBean.getFreeMemorySize: 1030758400
> [0.305s][trace][os,container] Path to /memory.usage_in_bytes is 
> /sys/fs/cgroup/memory/memory.usage_in_bytes
> [0.305s][trace][os,container] Memory Usage is: 42979328
> [0.306s][trace][os,container] Path to /memory.usage_in_bytes is 
> /sys/fs/cgroup/memory/memory.usage_in_bytes
> OperatingSystemMXBean.getFreePhysicalMemorySize: 
> [0.306s][trace][os,container] Memory Usage is: 42975232
> 1030762496
> OperatingSystemMXBean.getTotalSwapSpaceSize: 499122176
> 
> 
> java.lang.RuntimeException: 
> 'OperatingSystemMXBean\\.getFreePhysicalMemorySize: [1-9][0-9]+' missing from 
> stdout/stderr 
> 
>   at 
> jdk.test.lib.process.OutputAnalyzer.shouldMatch(OutputAnalyzer.java:306)
>   at 
> TestMemoryAwareness.testOperatingSystemMXBeanAwareness(TestMemoryAwareness.java:151)
>   at TestMemoryAwareness.main(TestMemoryAwareness.java:73)
>   at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>   at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.base/java.lang.reflect.Method.invoke(Method.java:564)
>   at 
> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
>   at java.base/java.lang.Thread.run(Thread.java:832)
> 
> Testing: Mach5 tier1-tier3 and open/test/hotspot/jtreg/containers/docker 
> tests passed. Tier4-tier6 tests are still running.
> 
> [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8226575/webrev.05 
> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8226575
> [3] CSR: https://bugs.openjdk.java.net/browse/JDK-8228428
> [4] https://bugs.openjdk.java.net/browse/JDK-8235522 
> 
> Thank you,
> Daniil
> 
> On 12/6/19, 1:38 PM, "Mandy Chung"  wrote:
> 
> 
> 
>On 12/6/19 5:59 AM, Bob Vandette wrote:
>>> On Dec 6, 2019, at 2:49 AM, David Holmes  wrote:
>>> 
>>> 
>>> src/jdk.managemen

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

eMatchingLine$1(SubSystem.java:116)
>>  at 
>> java.base/java.security.AccessController.doPrivileged(AccessController.java:554)
>> In getStringValue method the whole code block is now executed inside 
>> AccessController.doPrivileged() so we still need either catch
>> IOException inside this code block or convert this  block  to  
>> PrivilegedExceptionAction and then put AccessController.doPrivileged
>> call inside new try/catch Block to catch PrivilegedExceptionAction. The 
>> former approach looked more preferable.
>> Testing: Mach5 tier1-tier3 and open/test/hotspot/jtreg/containers/docker 
>> tests passed. Tier4-tier6 tests are still running.
>> [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8226575/webrev.04/
>> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8226575
>> [3] CSR: https://bugs.openjdk.java.net/browse/JDK-8228428
>> Thanks,
>> Daniil
>> On 12/5/19, 12:59 PM, "Mandy Chung"  wrote:
>>   On 12/5/19 12:50 PM, Bob Vandette wrote:
>> >
>> >>>> It may worth considering adding Metrics::getSwapLimit and
>> >>>> Metrics::getSwapUsage and move the computation to the 
>> implementation of
>> >>>> Metrics.  Bob may have an opinion.
>> >> There was no any new input regarding this so I decided to leave it 
>> unchanged.
>> > Sorry, I didn’t respond to this.  Since the calculation required for 
>> getFreeSwapSpaceSize requires retries
>> > due to the access of multiple changing values, I think it’s best to 
>> leave things as they are so the caller of
>> > these methods understands the limitations of the API.
>>  OK with me.
>> > Also, the fact that swap size metrics include memory sizes is fully 
>> documented in both the cgroup and docker
>> > online documentation so it’s probably best to be consistent.
>> >
>> >>>> Also it seems correct for the memory related methods to check if
>> >>>> (containerMetrics != null && containerMetrics.getMemoryLimit() >= 
>> 0).
>> >>>> BTW what does it mean if limit == 0?
>> >> Per Docker docs the minimum allowed value for  memory limit (--memory 
>> option) is 4 megabytes.
>> >> And if memory limit is unset the return value is -1.  Thus, in my 
>> understanding the value 0 is only possible
>> >> if something went wrong while retrieving this metric.
>> > That is true but shouldn’t you return -1 in that case?
>> >
>> > I originally thought it was ok to fall back to the host data for 0 
>> values but I think its better to return unavailable (-1)
>> > I think you might want to change all >= 0 to > 0 and return -1 if any 
>> of the values are 0.  This would be more consistent.
>>  +1
>>  The javadoc should be changed and returns -1 when it's unavailable 
>> and
>> the CSR should also be updated to reflect this.I'm sure Joe can
>> re-approve the CSR quickly when the fix is reviewed and approved.
>>  > You should only fall back to the original logic (host values) if 
>> container values are set to unlimited.
>> >
>> +1
>>  Mandy
>> 

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

In 
http://cr.openjdk.java.net/~dtitov/8226575/webrev.03/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java.sdiff.html

Shouldn’t you keep the IOException catch clauses in case the file is not found?


> On Dec 5, 2019, at 2:31 PM, Daniil Titov  wrote:
> 
> Hi Mandy and Bob,
> 
> Please review a new version of the webrev that addresses  the most of these 
> issues:
> 
> 1) The interface and spec [3] were updated to use default methods. CSR [3] 
> was re-approved.
> 
> 2) Security-sensitive operations in j.i.p.cgroupv1.Metrics and in 
> j.i.p.cgroupv1. SubSystem
>   were wrapped with doPrivileged
> 
> 3) getCpuLoad () method was optimized to fallback to  getSystemCpuLoad0  if 
> the cpuset is identical to the host's one. 
>   It uses sysconf(_SC_NPROCESSORS_CONF) to retrieve the number of CPUs 
> configured on the host . Testing with
>   different  --cpuset-cpus settings inside a Docker container proved  that it 
> always returns the same number of  hosts configured
>   CPUs regardless of --cpuset-cpus settings while the same settings affect 
> getEffectiveCpuSetCpus and getCpuSetCpus() metrics.
> 
>In addition, getCpuLoad () method  now returns -1 in the cases when quotas 
> are active but cpu usage and cpu period metrics are not available and
>   in the case when  for some reason it fails to retrieve a valid CPU load for 
> one of CPUs while iteration over them 

Shouldn't you do the same for getCpuLoad

149 int[] cpuSet = 
containerMetrics.getEffectiveCpuSetCpus();
150 if (cpuSet != null && cpuSet.length > 0) {

If cpuSet.length == 0?


> 
>>> CheckOperatingSystemMXBean.java
>>>System.out.println(String.format(...)) can simply be replaced with 
>>> System.out.format.
> I had to leave it unchanged since replacing  it with System.out.format 
> results in the tests instability as  it makes the trace output
> occasionally Intervene  here (the trace message sometimes is printed inside 
> this message)  and tests cannot find the expected
> pattern in the output.
> 
>>> It may worth considering adding Metrics::getSwapLimit and 
>>> Metrics::getSwapUsage and move the computation to the implementation of 
>>> Metrics.  Bob may have an opinion.
> 
> There was no any new input regarding this so I decided to leave it unchanged.

Sorry, I didn’t respond to this.  Since the calculation required for 
getFreeSwapSpaceSize requires retries
due to the access of multiple changing values, I think it’s best to leave 
things as they are so the caller of
these methods understands the limitations of the API.

Also, the fact that swap size metrics include memory sizes is fully documented 
in both the cgroup and docker
online documentation so it’s probably best to be consistent.

> 
>>> Also it seems correct for the memory related methods to check if 
>>> (containerMetrics != null && containerMetrics.getMemoryLimit() >= 0).  
>>> BTW what does it mean if limit == 0?
> 
> Per Docker docs the minimum allowed value for  memory limit (--memory option) 
> is 4 megabytes.
> And if memory limit is unset the return value is -1.  Thus, in my 
> understanding the value 0 is only possible
> if something went wrong while retrieving this metric.

That is true but shouldn’t you return -1 in that case?

I originally thought it was ok to fall back to the host data for 0 values but I 
think its better to return unavailable (-1)
I think you might want to change all >= 0 to > 0 and return -1 if any of the 
values are 0.  This would be more consistent.

You should only fall back to the original logic (host values) if container 
values are set to unlimited.

Bob.

> 
> Testing: Mach5 tier1-tier6 tests (that include 
> open/test/hotspot/jtreg/containers/docker  and : jdk_management  tests) 
> passed. 
> 
> [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8226575/webrev.03/ 
> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8226575
> [3] CSR: https://bugs.openjdk.java.net/browse/JDK-8228428
> 
> Thank you,
> Daniil
> 
> On 12/4/19, 4:09 PM, "Mandy Chung"  wrote:
> 
> 
> 
>On 12/3/19 9:40 PM, Daniil Titov wrote:
>> 
 Under what circumstance that limit or memLimit is < 0?
>> The memory limit metrics is not available if JVM runs on Linux host ( not in 
>> a docker container) or if a docker container was started without
>> specifying a memory limit ( without '--memory='  Docker option) . In latter 
>> there is no limit on how much memory the container can use and
>> it can use as much memory as the host's OS allows.
>> 
> 
>OK.  Please add a comment to the code.
> 
>It may worth considering adding Metrics::getSwapLimit and 
>Metrics::getSwapUsage and move the computation to the implementation of 
>Metrics.  Bob may have an opinion.
> 
>Also it seems correct for the memory related methods to check if 
>(containerMetrics != null && containerMetrics.getMemoryLimit() >= 0).  
>BTW what does it mean if limit == 0?
> 
 Is it worth  

Re: jmx-dev 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

> On Dec 4, 2019, at 8:32 AM, Bob Vandette  wrote:
> 
> 
>> On Dec 3, 2019, at 9:00 PM, Daniil Titov  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"  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  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
>>> 
>>> 
>> 
>> 
>> 
>> 
> 

Re: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

> On Dec 3, 2019, at 9:00 PM, Daniil Titov  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.

Bob.


> 
> Thank you,
> Daniil
> 
> On 12/3/19, 1:30 PM, "Bob Vandette"  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  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
>> 
>> 
> 
> 
> 
> 

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

[Bob Vandette]

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

Re: RFR(S): 8228960: [TESTBUG] containers/docker/TestJcmdWithSideCar.java: jcmd reports main class as 'Unknown'

[Bob Vandette]

Misha,

Looks good.  A couple of nits.

1. You might want to remove hotspot_containers section of ProblemList.txt since 
there are no bugs listed.

2. Can you make this timeout a constant just like TIME_TO_RUN_MAIN_PROCESS

  73 mainContainer.waitForMainMethodStart(5*1000);

3. assertIsAlive() is not used except for the commented out tests.  Do you 
think you’ll ultimately use
this method or is this left over from previous attempts?

222 public void assertIsAlive() throws Exception {


Bob.


> On Aug 29, 2019, at 11:41 AM, mikhailo.seledt...@oracle.com wrote:
> 
> I believe I need a second reviewer for this change. Could someone, please, 
> review this change version 2 ? (David already reviewed it).
> 
> http://cr.openjdk.java.net/~mseledtsov/8228960.02/
> 
> 
> Thank you in advance,
> 
> Misha
> 
> 
> On 8/26/19 12:32 PM, mikhailo.seledt...@oracle.com wrote:
>> Hi David,
>> 
>>   Thank you for review.
>> 
>> On 8/26/19 12:57 AM, David Holmes wrote:
>>> Hi Misha,
>>> 
>>> On 24/08/2019 3:21 am, mikhailo.seledt...@oracle.com wrote:
>>>> Finally got some time to work on this issue.
>>>> Since I have encountered problem using files for passing messages between 
>>>> a container and a test driver (due to permissions), I looked for 
>>>> alternative solutions. I am using the output of a container process to 
>>>> signal when the main method has started, and it works. This simplifies 
>>>> things quite a bit as well.
>>>> 
>>>> Normally, we use OutputAnalyzer test utility to collect the whole output 
>>>> once the process has completed, and then analyze the resulting output for 
>>>> "contains some string", match, etc. However, testutils/ProcessTools 
>>>> provides an API to consume the output as it is produced. I am using this 
>>>> API to detect when the main() method of the container has started.
>>> 
>>> That seems reasonable. Do we want to make the following change to minimise 
>>> unneeded output processing:
>>> 
>>>  private Consumer outputConsumer = s -> {
>>> !if (!mainMethodStarted && 
>>> s.contains(EventGeneratorLoop.MAIN_METHOD_STARTED)) {
>>>  System.out.println("MainContainer: setting 
>>> mainMethodStarted");
>>>  mainMethodStarted = true;
>>>  }
>>>  };
>> Thank you for the suggestion. I will update the code accordingly.
>>> 
>>>> Updated webrev:
>>>>  http://cr.openjdk.java.net/~mseledtsov/8228960.02/
>>> 
>>> Otherwise looks okay. Hopefully those other test cases will be enabled in 
>>> the not too distant future.
>> 
>> I hope so as well.
>> 
>> 
>> Thank you,
>> 
>> Misha
>> 
>>> 
>>> Thanks,
>>> David
>>> -
>>> 
>>>> 
>>>> Testing:
>>>> 
>>>>Ran the test on Linux-x64, various multiple nodes in a test cluster 50 
>>>> times - All PASS
>>>> 
>>>> 
>>>> Thank you,
>>>> 
>>>> Misha
>>>> 
>>>> On 8/13/19 2:05 PM, Bob Vandette wrote:
>>>>> 
>>>>>> On Aug 13, 2019, at 3:28 PM, mikhailo.seledt...@oracle.com wrote:
>>>>>> 
>>>>>> 
>>>>>> On 8/13/19 12:06 PM, Bob Vandette wrote:
>>>>>>>> On Aug 13, 2019, at 2:57 PM, mikhailo.seledt...@oracle.com wrote:
>>>>>>>> 
>>>>>>>> Hi Bob,
>>>>>>>> 
>>>>>>>>The workdir (JTwork/scratch) is created with the "test user" 
>>>>>>>> permissions. Let me try to place the "signal" file in /tmp instead, 
>>>>>>>> since /tmp should normally have a 777 permission on Linux.
>>>>>>> Aren’t you creating a file inside a docker container and then checking 
>>>>>>> for its existence outside of the container?
>>>>>> Correct
>>>>>>> Isn’t the root user running inside the container?
>>>>>> By default it is. But it still fails to create a file, for some reason. 
>>>>>> Can be related to selinux settings (for instance, see this article: 
>>>>>> https://stackoverflow.com/questions/24288616/permission-denied-on-accessing-host-directory-in-docker/31334443),
>>>>>>  I can not change t

Re: RFR(S): 8228960: [TESTBUG] containers/docker/TestJcmdWithSideCar.java: jcmd reports main class as 'Unknown'

[Bob Vandette]

> On Aug 13, 2019, at 3:28 PM, mikhailo.seledt...@oracle.com wrote:
> 
> 
> On 8/13/19 12:06 PM, Bob Vandette wrote:
>> 
>>> On Aug 13, 2019, at 2:57 PM, mikhailo.seledt...@oracle.com wrote:
>>> 
>>> Hi Bob,
>>> 
>>>   The workdir (JTwork/scratch) is created with the "test user" permissions. 
>>> Let me try to place the "signal" file in /tmp instead, since /tmp should 
>>> normally have a 777 permission on Linux.
>> Aren’t you creating a file inside a docker container and then checking for 
>> its existence outside of the container?
> Correct
>> Isn’t the root user running inside the container?
> 
> By default it is. But it still fails to create a file, for some reason. Can 
> be related to selinux settings (for instance, see this article: 
> https://stackoverflow.com/questions/24288616/permission-denied-on-accessing-host-directory-in-docker/31334443),
>  I can not change those.

Is your JTWork/scratch on an NFS mounted file system?  If this is the case then 
the problem is that root is equivalent to nobody on
mounted file systems and can’t create files unless the directory has 777 
permissions.  I just confirmed this.  You’d have to either run
the container test as test-user or change the scratch directory permission.

Bob.

> 
> My hope is that /tmp is configured to be accessed by a container engine as a 
> general purpose directory, hence I was thinking to try it out.
> 
>> 
>> Both processes don’t see the same /tmp right?   So that shouldn’t help.
> In my next experiment, I will map a /tmp from host to be a /host-tmp inside 
> the container (--volume /tmp:/host-tmp), then write a signal file to 
> /host-tmp.
>> 
>> If scratch has 777 permissions, anyone can create a file.
> scratch has  "rwxr-xr-x"
>> You have to be careful that you can clean up the
>> file from outside the container.  I’d make sure to create it with 777.
> 
> I do use deleteOnExit(), so it should work (unless the JVM crashes). I guess 
> I could add extra layer of safety here, and set the permissions to 777. Thank 
> you for advice.
> 
> 
> Thank you,
> 
> Misha
> 
>> 
>> Bob.
>> 
>>> If this works, I will have to add some unique number to the file name, 
>>> perhaps a PID of a child process.
>>> 
>>> I will try this, and let you know how it works.
>>> 
>>> 
>>> Thank you,
>>> 
>>> Misha
>>> 
>>> On 8/13/19 6:34 AM, Bob Vandette wrote:
>>>> Sorry, I just looked at the webrev and you are trying the approach I 
>>>> suggested.  I thought you
>>>> were trying to use file change notification.
>>>> 
>>>> Where does the workdir get created?  Does it have 777 permissions?
>>>> 
>>>> Bob.
>>>> 
>>>> 
>>>>> On Aug 13, 2019, at 9:29 AM, Bob Vandette  wrote:
>>>>> 
>>>>> What if you just poll for the creation of the file waiting some small 
>>>>> amount of time between polling with a maximum timeout.
>>>>> 
>>>>> Bob.
>>>>> 
>>>>> 
>>>>>> On Aug 12, 2019, at 8:22 PM, mikhailo.seledt...@oracle.com wrote:
>>>>>> 
>>>>>> Unfortunately, this approach does not seem to work on many of our test 
>>>>>> cluster machines. The creation of a "signal" file results in 
>>>>>> "PermissionDenied".
>>>>>> 
>>>>>> The possible reason is the selinux configuration, or some other 
>>>>>> permission related stuff. The container tries to create a new file on a 
>>>>>> mounted volume on a host system, but host system denies it. I will look 
>>>>>> a bit deeper into this, but I think this type of issue can be 
>>>>>> encountered on any automated test system. Hence, we may have to abandon 
>>>>>> this approach.
>>>>>> 
>>>>>> 
>>>>>> Thanks,
>>>>>> 
>>>>>> Misha
>>>>>> 
>>>>>> 
>>>>>> On 8/12/19 3:59 PM, mikhailo.seledt...@oracle.com wrote:
>>>>>>> Here is an updated webrev: 
>>>>>>> http://cr.openjdk.java.net/~mseledtsov/8228960.01/
>>>>>>> 
>>>>>>> I am using a simple file-based mechanism to communicate between the 
>>>>>>> processes. The "EventGeneratorLoop" process creates a specific "signa

Re: RFR(S): 8228960: [TESTBUG] containers/docker/TestJcmdWithSideCar.java: jcmd reports main class as 'Unknown'

[Bob Vandette]

> On Aug 13, 2019, at 2:57 PM, mikhailo.seledt...@oracle.com wrote:
> 
> Hi Bob,
> 
>   The workdir (JTwork/scratch) is created with the "test user" permissions. 
> Let me try to place the "signal" file in /tmp instead, since /tmp should 
> normally have a 777 permission on Linux.

Aren’t you creating a file inside a docker container and then checking for its 
existence outside of the container?
Isn’t the root user running inside the container?

Both processes don’t see the same /tmp right?   So that shouldn’t help.

If scratch has 777 permissions, anyone can create a file.  You have to be 
careful that you can clean up the
file from outside the container.  I’d make sure to create it with 777.

Bob.

> 
> If this works, I will have to add some unique number to the file name, 
> perhaps a PID of a child process.
> 
> I will try this, and let you know how it works.
> 
> 
> Thank you,
> 
> Misha
> 
> On 8/13/19 6:34 AM, Bob Vandette wrote:
>> Sorry, I just looked at the webrev and you are trying the approach I 
>> suggested.  I thought you
>> were trying to use file change notification.
>> 
>> Where does the workdir get created?  Does it have 777 permissions?
>> 
>> Bob.
>> 
>> 
>>> On Aug 13, 2019, at 9:29 AM, Bob Vandette  wrote:
>>> 
>>> What if you just poll for the creation of the file waiting some small 
>>> amount of time between polling with a maximum timeout.
>>> 
>>> Bob.
>>> 
>>> 
>>>> On Aug 12, 2019, at 8:22 PM, mikhailo.seledt...@oracle.com wrote:
>>>> 
>>>> Unfortunately, this approach does not seem to work on many of our test 
>>>> cluster machines. The creation of a "signal" file results in 
>>>> "PermissionDenied".
>>>> 
>>>> The possible reason is the selinux configuration, or some other permission 
>>>> related stuff. The container tries to create a new file on a mounted 
>>>> volume on a host system, but host system denies it. I will look a bit 
>>>> deeper into this, but I think this type of issue can be encountered on any 
>>>> automated test system. Hence, we may have to abandon this approach.
>>>> 
>>>> 
>>>> Thanks,
>>>> 
>>>> Misha
>>>> 
>>>> 
>>>> On 8/12/19 3:59 PM, mikhailo.seledt...@oracle.com wrote:
>>>>> Here is an updated webrev: 
>>>>> http://cr.openjdk.java.net/~mseledtsov/8228960.01/
>>>>> 
>>>>> I am using a simple file-based mechanism to communicate between the 
>>>>> processes. The "EventGeneratorLoop" process creates a specific "signal" 
>>>>> file on a shared mounted volume, while the main test process waits  for 
>>>>> the file to exist before running the test cases.
>>>>> 
>>>>> Passes on Linux-x64 Docker-enabled host. Testing in the test cluster is 
>>>>> in progress.
>>>>> 
>>>>> 
>>>>> Thank you,
>>>>> 
>>>>> Misha
>>>>> 
>>>>> On 8/7/19 5:11 PM, David Holmes wrote:
>>>>>> On 8/08/2019 9:04 am, Mikhailo Seledtsov wrote:
>>>>>>> Hi Severin, Bob,
>>>>>>> 
>>>>>>>   Thank you for reviewing the code.
>>>>>>> 
>>>>>>> On 8/7/19, 11:38 AM, Bob Vandette wrote:
>>>>>>>> Can’t you come up with a better way of synchronizing the test by 
>>>>>>>> possibly writing a
>>>>>>>> file and waiting for it to exist with a timeout?
>>>>>>> I will try out this approach.
>>>>>> This seems like a fundamental problem with jcmd - so cc'ing 
>>>>>> serviceability-dev.
>>>>>> 
>>>>>> But I'm pretty sure they recently addressed a similar issue with the 
>>>>>> premature sending of the attach signal?
>>>>>> 
>>>>>> David
>>>>>> -
>>>>>> 
>>>>>>> Thanks,
>>>>>>> Misha
>>>>>>>> Isn’t there a shared volume between the two
>>>>>>>> processes?
>>>>>>>> 
>>>>>>>> We’ve been fighting test reliability for a while now.  I can only hope 
>>>>>>>> we’re getting
>>>>>>>> to the end.
>>>>>>>> 
>>

Re: RFR(S): 8228960: [TESTBUG] containers/docker/TestJcmdWithSideCar.java: jcmd reports main class as 'Unknown'

[Bob Vandette]

Sorry, I just looked at the webrev and you are trying the approach I suggested. 
 I thought you
were trying to use file change notification.

Where does the workdir get created?  Does it have 777 permissions?

Bob.


> On Aug 13, 2019, at 9:29 AM, Bob Vandette  wrote:
> 
> What if you just poll for the creation of the file waiting some small amount 
> of time between polling with a maximum timeout.
> 
> Bob.
> 
> 
>> On Aug 12, 2019, at 8:22 PM, mikhailo.seledt...@oracle.com wrote:
>> 
>> Unfortunately, this approach does not seem to work on many of our test 
>> cluster machines. The creation of a "signal" file results in 
>> "PermissionDenied".
>> 
>> The possible reason is the selinux configuration, or some other permission 
>> related stuff. The container tries to create a new file on a mounted volume 
>> on a host system, but host system denies it. I will look a bit deeper into 
>> this, but I think this type of issue can be encountered on any automated 
>> test system. Hence, we may have to abandon this approach.
>> 
>> 
>> Thanks,
>> 
>> Misha
>> 
>> 
>> On 8/12/19 3:59 PM, mikhailo.seledt...@oracle.com wrote:
>>> Here is an updated webrev: 
>>> http://cr.openjdk.java.net/~mseledtsov/8228960.01/
>>> 
>>> I am using a simple file-based mechanism to communicate between the 
>>> processes. The "EventGeneratorLoop" process creates a specific "signal" 
>>> file on a shared mounted volume, while the main test process waits  for the 
>>> file to exist before running the test cases.
>>> 
>>> Passes on Linux-x64 Docker-enabled host. Testing in the test cluster is in 
>>> progress.
>>> 
>>> 
>>> Thank you,
>>> 
>>> Misha
>>> 
>>> On 8/7/19 5:11 PM, David Holmes wrote:
>>>> On 8/08/2019 9:04 am, Mikhailo Seledtsov wrote:
>>>>> Hi Severin, Bob,
>>>>> 
>>>>>   Thank you for reviewing the code.
>>>>> 
>>>>> On 8/7/19, 11:38 AM, Bob Vandette wrote:
>>>>>> Can’t you come up with a better way of synchronizing the test by 
>>>>>> possibly writing a
>>>>>> file and waiting for it to exist with a timeout?
>>>>> I will try out this approach.
>>>> 
>>>> This seems like a fundamental problem with jcmd - so cc'ing 
>>>> serviceability-dev.
>>>> 
>>>> But I'm pretty sure they recently addressed a similar issue with the 
>>>> premature sending of the attach signal?
>>>> 
>>>> David
>>>> -
>>>> 
>>>>> Thanks,
>>>>> Misha
>>>>>> Isn’t there a shared volume between the two
>>>>>> processes?
>>>>>> 
>>>>>> We’ve been fighting test reliability for a while now.  I can only hope 
>>>>>> we’re getting
>>>>>> to the end.
>>>>>> 
>>>>>> Bob.
>>>>>> 
>>>>>>> On Aug 7, 2019, at 2:18 PM, Severin Gehwolf  wrote:
>>>>>>> 
>>>>>>> Hi Misha,
>>>>>>> 
>>>>>>> On Tue, 2019-08-06 at 20:17 -0700, mikhailo.seledt...@oracle.com wrote:
>>>>>>>> Please review this change that fixes a container test 
>>>>>>>> TestJcmdWithSideCar.
>>>>>>>> 
>>>>>>>> My investigation indicated that a root cause for this failure is:
>>>>>>>> JCMD -l shows 'Unknown' for class name because the main class has not
>>>>>>>> been loaded yet.
>>>>>>>> The target test JVM has started, it is initializing, but has not loaded
>>>>>>>> the main test class.
>>>>>>> That's what I've found too.
>>>>>>> 
>>>>>>>> The proposed solution is to try 'jcmd -l' several times, with a short
>>>>>>>> sleep in between.
>>>>>>> Thread.sleep() isn't great, but I'm not sure there is an alternative.
>>>>>>> 
>>>>>>>> Also I have commented out the testCase02() due to another bug:
>>>>>>>> "JDK-8228850: jhsdb jinfo fails with ClassCastException:
>>>>>>>> s.j.h.oops.TypeArray cannot be cast to s.j.h.oops.Instance",
>>>>>>>> which is not a test bug. IMO, it is better to run the test and skip a
>>>>>>>> sub-case than to skip the entire test.
>>>>>>>> 
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8228960
>>>>>>>> Webrev: http://cr.openjdk.java.net/~mseledtsov/8228960.00/
>>>>>>> Looks OK to me.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Severin
>>>>>>> 
> 

Re: RFR(S): 8228960: [TESTBUG] containers/docker/TestJcmdWithSideCar.java: jcmd reports main class as 'Unknown'

[Bob Vandette]

What if you just poll for the creation of the file waiting some small amount of 
time between polling with a maximum timeout.

Bob.


> On Aug 12, 2019, at 8:22 PM, mikhailo.seledt...@oracle.com wrote:
> 
> Unfortunately, this approach does not seem to work on many of our test 
> cluster machines. The creation of a "signal" file results in 
> "PermissionDenied".
> 
> The possible reason is the selinux configuration, or some other permission 
> related stuff. The container tries to create a new file on a mounted volume 
> on a host system, but host system denies it. I will look a bit deeper into 
> this, but I think this type of issue can be encountered on any automated test 
> system. Hence, we may have to abandon this approach.
> 
> 
> Thanks,
> 
> Misha
> 
> 
> On 8/12/19 3:59 PM, mikhailo.seledt...@oracle.com wrote:
>> Here is an updated webrev: http://cr.openjdk.java.net/~mseledtsov/8228960.01/
>> 
>> I am using a simple file-based mechanism to communicate between the 
>> processes. The "EventGeneratorLoop" process creates a specific "signal" file 
>> on a shared mounted volume, while the main test process waits  for the file 
>> to exist before running the test cases.
>> 
>> Passes on Linux-x64 Docker-enabled host. Testing in the test cluster is in 
>> progress.
>> 
>> 
>> Thank you,
>> 
>> Misha
>> 
>> On 8/7/19 5:11 PM, David Holmes wrote:
>>> On 8/08/2019 9:04 am, Mikhailo Seledtsov wrote:
>>>> Hi Severin, Bob,
>>>> 
>>>>Thank you for reviewing the code.
>>>> 
>>>> On 8/7/19, 11:38 AM, Bob Vandette wrote:
>>>>> Can’t you come up with a better way of synchronizing the test by possibly 
>>>>> writing a
>>>>> file and waiting for it to exist with a timeout?
>>>> I will try out this approach.
>>> 
>>> This seems like a fundamental problem with jcmd - so cc'ing 
>>> serviceability-dev.
>>> 
>>> But I'm pretty sure they recently addressed a similar issue with the 
>>> premature sending of the attach signal?
>>> 
>>> David
>>> -
>>> 
>>>> Thanks,
>>>> Misha
>>>>> Isn’t there a shared volume between the two
>>>>> processes?
>>>>> 
>>>>> We’ve been fighting test reliability for a while now.  I can only hope 
>>>>> we’re getting
>>>>> to the end.
>>>>> 
>>>>> Bob.
>>>>> 
>>>>>> On Aug 7, 2019, at 2:18 PM, Severin Gehwolf  wrote:
>>>>>> 
>>>>>> Hi Misha,
>>>>>> 
>>>>>> On Tue, 2019-08-06 at 20:17 -0700, mikhailo.seledt...@oracle.com wrote:
>>>>>>> Please review this change that fixes a container test 
>>>>>>> TestJcmdWithSideCar.
>>>>>>> 
>>>>>>> My investigation indicated that a root cause for this failure is:
>>>>>>> JCMD -l shows 'Unknown' for class name because the main class has not
>>>>>>> been loaded yet.
>>>>>>> The target test JVM has started, it is initializing, but has not loaded
>>>>>>> the main test class.
>>>>>> That's what I've found too.
>>>>>> 
>>>>>>> The proposed solution is to try 'jcmd -l' several times, with a short
>>>>>>> sleep in between.
>>>>>> Thread.sleep() isn't great, but I'm not sure there is an alternative.
>>>>>> 
>>>>>>> Also I have commented out the testCase02() due to another bug:
>>>>>>> "JDK-8228850: jhsdb jinfo fails with ClassCastException:
>>>>>>> s.j.h.oops.TypeArray cannot be cast to s.j.h.oops.Instance",
>>>>>>> which is not a test bug. IMO, it is better to run the test and skip a
>>>>>>> sub-case than to skip the entire test.
>>>>>>> 
>>>>>>>  JBS: https://bugs.openjdk.java.net/browse/JDK-8228960
>>>>>>>  Webrev: http://cr.openjdk.java.net/~mseledtsov/8228960.00/
>>>>>> Looks OK to me.
>>>>>> 
>>>>>> Thanks,
>>>>>> Severin
>>>>>> 

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

> On Jul 11, 2019, at 1:18 PM, Andrew Azores  wrote:
> 
> Hi Bob (and all),
> 
> On Fri, 2019-06-21 at 08:50 -0400, Bob Vandette wrote:
>>> 
>> Fixing the existing OS Mbean is pretty straight forward.  Just have
>> each method call the new Metrics API, check for error returns
>> in case this API is not supported on this platform and if success,
>> return result, otherwise call existing logic.
>> 
> 
> I've had free time to circle back to this issue lately, so I've
> prepared a patch which modifies the OperatingSystemMXBean as you've
> described here. This seems to work perfectly well for
> getTotalPhysicalMemorySize, getFreePhysicalMemorySize, and
> getTotalSwapSpaceSize.


> getFreeSwapSpaceSize looks at a glance to be
> implementable by comparing Metrics' getMemoryAndSwapUsage,
> getMemoryUsage, and getMemoryAndSwap limit, but the implementation
> would only provide an approximation (and not necessarily accurately)
> since the amount of memory/swap used would likely change in between or
> due to subsequent calls to the Metrics API.

That one appears a bit tricky to get right.

Assuming memory and swap limits are set, returning getMemoryAndSwapLimit -  
getMemoryLimit - getMemoryAndSwapUsage.
Since the limits are constant, there’s no issue with variability between the 
two calls.

However, if the Swap limit is unlimited then I’d revert to the original logic 
since the limit is
the hosts when swap is unlimited.

One problem I see if that in cgroups, memory.swappiness can alter or disable 
swapping.
I didn’t provide a Metrics API that returns this value or take this into 
account in
the result of getMemoryAndSwapLimit.

> 
> Also, I'm not sure that there are reasonable equivalents for all of the
> other metrics implemented in
> jdk.management/unix/classes/com/sun/management/internal/OperatingSystem
> Impl.java available from Metrics or elsewhere which make sense in a
> containerized environment (ex. getCommittedVirtualMemorySize).

We’d have to experiment with this but since this is actual committed memory for
the current process, I would expect the existing logic to be accurate.

> 
> So, before I proceed much further with this patch, does anyone have

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

Here you go.

https://bugs.openjdk.java.net/browse/JDK-8226575 
<https://bugs.openjdk.java.net/browse/JDK-8226575>

Thanks,
Bob.

> On Jun 21, 2019, at 9:08 AM, Severin Gehwolf  wrote:
> 
> Hi Bob,
> 
> On Fri, 2019-06-21 at 08:56 -0400, Bob Vandette wrote:
>>> On Jun 21, 2019, at 4:22 AM, Severin Gehwolf  wrote:
>>> 
>>> Hi Bob,
>>> 
>>> On Thu, 2019-06-20 at 10:16 -0400, Bob Vandette wrote:
>>>> Hi Andrew,
>>>> 
>>>> I am aware of the limitations of the OperatingSystemMXBean and was
>>>> hoping to address these limitations during the implementation of
>>>> https://bugs.openjdk.java.net/browse/JDK-8199944.
>>>> 
>>>> It would be helpful if you feel this is important to add some votes
>>>> to this issue.
>>> 
>>> It seems strange that the getAvailableProcessors() returns the
>>> container limit, while the memory limits are for the physical host. If
>>> anything, shouldn't they agree (both physical host or both container
>>> limits)?
>>> 
>>> When I briefly looked into it initially it seems to be a side-effect of
>>> what is being used by the JDK code implementation-wise. IIRC
>>> getAvailableProcessors() uses a runtime call. Memory reporting has it's
>>> own logic[1], thus not reporting the container limit.
>>> 
>>> This seems weird from a consistency perspective. Thoughts?
>>> 
>>> If I understand you correctly, you are arguing that container reporting
>>> should go into its own MX bean. On the other hand, CPU reporting for
>>> the OS MX bean is container aware already. That makes me believe we
>>> should rather make this consistent before evaluating a new MX bean.
>> 
>> You make a good point.  I’ll split the enhancement and add a bug to fix the
>> current MX Bean since this is pretty easy to do.
> 
> Sounds great! Please let me know once the OS MX bean bug has been
> created. I'll then assign it to myself and help Andrew through the
> process of getting it fixed.
> 
> Thanks,
> Severin
> 
>> Bob.
>> 
>>> Thanks,
>>> Severin
>>> 
>>> [1] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/1c242c2d037f/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#l365
>>> 
>>> 
>>>> Bob.
>>>> 
>>>> 
>>>>> On Jun 20, 2019, at 9:43 AM, Andrew Azores 
>>>>> wrote:
>>>>> 
>>>>> Hi all,
>>>>> 
>>>>> Apologies if this is not the most appropriate list, in which case
>>>>> please direct me where to go.
>>>>> 
>>>>> I've noticed a surprising result from the
>>>>> com.sun.management.OperatingSystemMXBean implementation when
>>>>> running in
>>>>> a containerized (specifically, using Docker on Linux) environment.
>>>>> The
>>>>> bean appears to be container-aware for processors, in that running
>>>>> with
>>>>> Docker option `--cpus 1.0` for example, on a multicore system, will
>>>>> cause both java.lang.Runtime#availableProcessors and
>>>>> java.lang.management.OperatingSystemMXBean#getAvailableProcessors /
>>>>> com.sun.management.OperatingSystemMXBean#getAvailableProcessors to
>>>>> return 1. However, the Docker option `--memory 100M` (or any other
>>>>> limit value) is not reflected in the value returned by
>>>>> com.sun.management.OperatingSystemMXBeam#getTotalPhysicalMemorySize
>>>>> ,
>>>>> and instead the returned value is still the total physical memory
>>>>> of
>>>>> the host machine - of which only a small portion may actually be
>>>>> available to the "Operating System" of the JVM. Similarly for the
>>>>> methods regarding free physical memory, total swap, and free swap.
>>>>> 
>>>>> I have attached a patch which adds a small reproducer to the
>>>>> existing
>>>>> MemoryAwareness test.
>>>>> 
>>>>> This seems like a bug to me, since if the imposed container limit
>>>>> on
>>>>> processors as a resource is included as part of the "Operating
>>>>> System"
>>>>> resource reporting, then surely memory resources should be reported
>>>>> the
>>>>> same way. As I said, I found the current behaviour quite
>>>>> surprising.
>>>>> 
>>>>> -- 
>>>>> Andrew Azores
>>>>> Software Engineer, OpenJDK Team
>>>>> Red Hat
>>>>> 
> 

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

> On Jun 21, 2019, at 4:22 AM, Severin Gehwolf  wrote:
> 
> Hi Bob,
> 
> On Thu, 2019-06-20 at 10:16 -0400, Bob Vandette wrote:
>> Hi Andrew,
>> 
>> I am aware of the limitations of the OperatingSystemMXBean and was
>> hoping to address these limitations during the implementation of
>> https://bugs.openjdk.java.net/browse/JDK-8199944.
>> 
>> It would be helpful if you feel this is important to add some votes
>> to this issue.
> 
> It seems strange that the getAvailableProcessors() returns the
> container limit, while the memory limits are for the physical host. If
> anything, shouldn't they agree (both physical host or both container
> limits)?
> 
> When I briefly looked into it initially it seems to be a side-effect of
> what is being used by the JDK code implementation-wise. IIRC
> getAvailableProcessors() uses a runtime call. Memory reporting has it's
> own logic[1], thus not reporting the container limit.
> 
> This seems weird from a consistency perspective. Thoughts?
> 
> If I understand you correctly, you are arguing that container reporting
> should go into its own MX bean. On the other hand, CPU reporting for
> the OS MX bean is container aware already. That makes me believe we
> should rather make this consistent before evaluating a new MX bean.

You make a good point.  I’ll split the enhancement and add a bug to fix the
current MX Bean since this is pretty easy to do.

Bob.

> 
> Thanks,
> Severin
> 
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/1c242c2d037f/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#l365
> 
> 
>> Bob.
>> 
>> 
>>> On Jun 20, 2019, at 9:43 AM, Andrew Azores 
>>> wrote:
>>> 
>>> Hi all,
>>> 
>>> Apologies if this is not the most appropriate list, in which case
>>> please direct me where to go.
>>> 
>>> I've noticed a surprising result from the
>>> com.sun.management.OperatingSystemMXBean implementation when
>>> running in
>>> a containerized (specifically, using Docker on Linux) environment.
>>> The
>>> bean appears to be container-aware for processors, in that running
>>> with
>>> Docker option `--cpus 1.0` for example, on a multicore system, will
>>> cause both java.lang.Runtime#availableProcessors and
>>> java.lang.management.OperatingSystemMXBean#getAvailableProcessors /
>>> com.sun.management.OperatingSystemMXBean#getAvailableProcessors to
>>> return 1. However, the Docker option `--memory 100M` (or any other
>>> limit value) is not reflected in the value returned by
>>> com.sun.management.OperatingSystemMXBeam#getTotalPhysicalMemorySize
>>> ,
>>> and instead the returned value is still the total physical memory
>>> of
>>> the host machine - of which only a small portion may actually be
>>> available to the "Operating System" of the JVM. Similarly for the
>>> methods regarding free physical memory, total swap, and free swap.
>>> 
>>> I have attached a patch which adds a small reproducer to the
>>> existing
>>> MemoryAwareness test.
>>> 
>>> This seems like a bug to me, since if the imposed container limit
>>> on
>>> processors as a resource is included as part of the "Operating
>>> System"
>>> resource reporting, then surely memory resources should be reported
>>> the
>>> same way. As I said, I found the current behaviour quite
>>> surprising.
>>> 
>>> -- 
>>> Andrew Azores
>>> Software Engineer, OpenJDK Team
>>> Red Hat
>>> 
>> 
>> 
> 

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

> On Jun 21, 2019, at 6:06 AM, Mario Torre  
> wrote:
> 
> The way I understood the bug report is a two fold approach, i.e. fix the os 
> bean and *possibly* add a container one or extend it to add more data.
Correct.  We could file a separate bug to just correct the OS Bean but I was 
hoping to add access to the new Container Metrics information
at the same time.

> 
> I agree with you and Andrew that the current OS Bean returns wrong 
> information, this should be fixed in any case I think.
Yes.

> 
> Bob, do you have some design ideas to share regarding the new interface? I 
> think this may be an interesting project to pick up, even for students 
> wanting to write a thesis, as it seems time is a limiting factor here for you 
> to work on that?

Fixing the existing OS Mbean is pretty straight forward.  Just have each method 
call the new Metrics API, check for error returns
in case this API is not supported on this platform and if success, return 
result, otherwise call existing logic.

Adding new OS access or a new Container Mbean is a bit more involved.

Since this is a public API, we’d need to take a look at at least one more OS 
(possibly Windows) and
decide which of the Metrics we want to expose.  I focused on Linux since this 
is the primary OS used
in container runtimes (even on Windows).  

Once you have the list of Metrics and Configuration data, Add a new Container 
MBean modeled
after the accessors in the Metrics API.

jdk/open/src/java.base/share/classes/jdk/internal/platform/Metrics.java

You’d need to ensure proper javadocs are available and jtreg tests would need 
to be written to
validate the new APIs.

Bob.


> 
> Cheers,
> Mario
> 
> On Fri 21. Jun 2019 at 10:53, Severin Gehwolf  wrote:
> Hi Bob,
> 
> On Thu, 2019-06-20 at 10:16 -0400, Bob Vandette wrote:
> > Hi Andrew,
> > 
> > I am aware of the limitations of the OperatingSystemMXBean and was
> > hoping to address these limitations during the implementation of
> > https://bugs.openjdk.java.net/browse/JDK-8199944.
> > 
> > It would be helpful if you feel this is important to add some votes
> > to this issue.
> 
> It seems strange that the getAvailableProcessors() returns the
> container limit, while the memory limits are for the physical host. If
> anything, shouldn't they agree (both physical host or both container
> limits)?
> 
> When I briefly looked into it initially it seems to be a side-effect of
> what is being used by the JDK code implementation-wise. IIRC
> getAvailableProcessors() uses a runtime call. Memory reporting has it's
> own logic[1], thus not reporting the container limit.
> 
> This seems weird from a consistency perspective. Thoughts?
> 
> If I understand you correctly, you are arguing that container reporting
> should go into its own MX bean. On the other hand, CPU reporting for
> the OS MX bean is container aware already. That makes me believe we
> should rather make this consistent before evaluating a new MX bean.
> 
> Thanks,
> Severin
> 
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/1c242c2d037f/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#l365
> 
> 
> > Bob.
> > 
> > 
> > > On Jun 20, 2019, at 9:43 AM, Andrew Azores 
> > > wrote:
> > > 
> > > Hi all,
> > > 
> > > Apologies if this is not the most appropriate list, in which case
> > > please direct me where to go.
> > > 
> > > I've noticed a surprising result from the
> > > com.sun.management.OperatingSystemMXBean implementation when
> > > running in
> > > a containerized (specifically, using Docker on Linux) environment.
> > > The
> > > bean appears to be container-aware for processors, in that running
> > > with
> > > Docker option `--cpus 1.0` for example, on a multicore system, will
> > > cause both java.lang.Runtime#availableProcessors and
> > > java.lang.management.OperatingSystemMXBean#getAvailableProcessors /
> > > com.sun.management.OperatingSystemMXBean#getAvailableProcessors to
> > > return 1. However, the Docker option `--memory 100M` (or any other
> > > limit value) is not reflected in the value returned by
> > > com.sun.management.OperatingSystemMXBeam#getTotalPhysicalMemorySize
> > > ,
> > > and instead the returned value is still the total physical memory
> > > of
> > > the host machine - of which only a small portion may actually be
> > > available to the "Operating System" of the JVM. Similarly for the
> > > methods regarding free physical memory, total swap, and free swap.
> > > 
> > > I have attached a patch which adds a small r

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

I agree with you Mario.  When I originally designed the 
jdk.internal.platform.Metrics
API, I was considering providing both a Host and Container Metrics 
implementation.
The problem is that the primary goal of containers is to provide isolation 
(hiding access to
the host) and it would be very difficult to provide reliable forms of both 
metrics from within
a container.  The Linux kernel does leak some host metric information but not 
everything.
This is why I decided to focus on the container configuration and metric data.  
If Host
metrics are of interest you can always just run Java outside of a container.

Bob.

> On Jun 21, 2019, at 8:27 AM, Mario Torre  
> wrote:
> 
> Hi Kirk,
> 
> I think I understand what you mean, however then the OS Bean should be 
> consistent regarding CPU information as well.
> 
> I think I remember why this was fixed the way it is now was because of 
> incorrect behavior during GC configuration, or something along those lines, 
> so I guess we would need two APIs after all to give tooling a way to sneak 
> into the actual hardware properties.
> 
> I would guess, however, that from the point of view of a containerised VM its 
> OS is the container not the bare metal (or virtualized metal perhaps), so 
> tooling would need to check specifically for a separate bean.
> 
> That could be indicated via a property in the OS Bean (if it’s not the case 
> already).
> 
> Nevertheless, I think consistency in the OS Bean should be achieved.
> 
> Cheers,
> Mario 
> 
> On Fri 21. Jun 2019 at 13:23, Kirk Pepperdine  <mailto:kirk.pepperd...@gmail.com>> wrote:
> Hi Mario,
> 
> I don’t believe the MBean returns the wrong information. Is it not that the 
> calls by-pass the container? Would it not be more appropriate to add a 
> container aware mean? From a tooling perspective it’s a mistake to completely 
> seal the JVM away from the hardware as it makes certain diagnostics more 
> difficult to perform.
> 
> Kind regards,
> Kirk
> 
> 
>> On Jun 21, 2019, at 6:06 AM, Mario Torre > <mailto:neugens.limasoftw...@gmail.com>> wrote:
>> 
>> The way I understood the bug report is a two fold approach, i.e. fix the os 
>> bean and *possibly* add a container one or extend it to add more data.
>> 
>> I agree with you and Andrew that the current OS Bean returns wrong 
>> information, this should be fixed in any case I think.
>> 
>> Bob, do you have some design ideas to share regarding the new interface? I 
>> think this may be an interesting project to pick up, even for students 
>> wanting to write a thesis, as it seems time is a limiting factor here for 
>> you to work on that?
>> 
>> Cheers,
>> Mario
>> 
>> On Fri 21. Jun 2019 at 10:53, Severin Gehwolf > <mailto:sgehw...@redhat.com>> wrote:
>> Hi Bob,
>> 
>> On Thu, 2019-06-20 at 10:16 -0400, Bob Vandette wrote:
>> > Hi Andrew,
>> > 
>> > I am aware of the limitations of the OperatingSystemMXBean and was
>> > hoping to address these limitations during the implementation of
>> > https://bugs.openjdk.java.net/browse/JDK-8199944 
>> > <https://bugs.openjdk.java.net/browse/JDK-8199944>.
>> > 
>> > It would be helpful if you feel this is important to add some votes
>> > to this issue.
>> 
>> It seems strange that the getAvailableProcessors() returns the
>> container limit, while the memory limits are for the physical host. If
>> anything, shouldn't they agree (both physical host or both container
>> limits)?
>> 
>> When I briefly looked into it initially it seems to be a side-effect of
>> what is being used by the JDK code implementation-wise. IIRC
>> getAvailableProcessors() uses a runtime call. Memory reporting has it's
>> own logic[1], thus not reporting the container limit.
>> 
>> This seems weird from a consistency perspective. Thoughts?
>> 
>> If I understand you correctly, you are arguing that container reporting
>> should go into its own MX bean. On the other hand, CPU reporting for
>> the OS MX bean is container aware already. That makes me believe we
>> should rather make this consistent before evaluating a new MX bean.
>> 
>> Thanks,
>> Severin
>> 
>> [1] 
>> http://hg.openjdk.java.net/jdk/jdk/file/1c242c2d037f/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#l365
>>  
>> <http://hg.openjdk.java.net/jdk/jdk/file/1c242c2d037f/src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c#l365>
>> 
>> 
>> > Bob.
>> > 
>> > 
>> > > On Jun 20, 2019, at 9:43 AM, An

Re: OperatingSystemMXBean unaware of container memory limits

[Bob Vandette]

Hi Andrew,

I am aware of the limitations of the OperatingSystemMXBean and was
hoping to address these limitations during the implementation of
https://bugs.openjdk.java.net/browse/JDK-8199944 
.

It would be helpful if you feel this is important to add some votes to this 
issue.

Bob.


> On Jun 20, 2019, at 9:43 AM, Andrew Azores  wrote:
> 
> Hi all,
> 
> Apologies if this is not the most appropriate list, in which case
> please direct me where to go.
> 
> I've noticed a surprising result from the
> com.sun.management.OperatingSystemMXBean implementation when running in
> a containerized (specifically, using Docker on Linux) environment. The
> bean appears to be container-aware for processors, in that running with
> Docker option `--cpus 1.0` for example, on a multicore system, will
> cause both java.lang.Runtime#availableProcessors and
> java.lang.management.OperatingSystemMXBean#getAvailableProcessors /
> com.sun.management.OperatingSystemMXBean#getAvailableProcessors to
> return 1. However, the Docker option `--memory 100M` (or any other
> limit value) is not reflected in the value returned by
> com.sun.management.OperatingSystemMXBeam#getTotalPhysicalMemorySize,
> and instead the returned value is still the total physical memory of
> the host machine - of which only a small portion may actually be
> available to the "Operating System" of the JVM. Similarly for the
> methods regarding free physical memory, total swap, and free swap.
> 
> I have attached a patch which adds a small reproducer to the existing
> MemoryAwareness test.
> 
> This seems like a bug to me, since if the imposed container limit on
> processors as a resource is included as part of the "Operating System"
> resource reporting, then surely memory resources should be reported the
> same way. As I said, I found the current behaviour quite surprising.
> 
> -- 
> Andrew Azores
> Software Engineer, OpenJDK Team
> Red Hat
> 

Re: RFR: 8217338: [Containers] Improve systemd slice memory limit support

[Bob Vandette]

Sorry for the delay.  The update looks good.

Bob.


> On Mar 25, 2019, at 1:30 PM, Severin Gehwolf  wrote:
> 
> On Fri, 2019-03-22 at 14:25 -0400, Bob Vandette wrote:
>> Could you maybe combine subsystem_file_contents with 
>> subsystem_file_line_contents
>> by adding an additional argument?
> 
> Done: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217338/05/webrev/
> 
> Thanks,
> Severin
> 

Re: RFR: 8217338: [Containers] Improve systemd slice memory limit support

[Bob Vandette]

Is there ever a situation where the memory.limit_in_bytes could be unlimited 
but the
hierarchical_memory_limit is not?

Could you maybe combine subsystem_file_contents with 
subsystem_file_line_contents
by adding an additional argument?



BTW:  I found another problem with the mountinfo/cgroup parsing that impacts the
container tests.

I don’t know why it only caused a failure on one of my systems.  I’m going to
file another bug.  You might want to test with these changes to make sure
your looking at the correct subsystem files.


diff --git 
a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java 
b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
--- a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
+++ b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
@@ -60,7 +60,7 @@
 path = mountPoint;
 }
 else {
-if (root.indexOf(cgroupPath) == 0) {
+if (cgroupPath.indexOf(root) == 0) {
 if (cgroupPath.length() > root.length()) {
 String cgroupSubstr = 
cgroupPath.substring(root.length());
 path = mountPoint + cgroupSubstr;
diff --git a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java 
b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
--- a/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
+++ b/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java
@@ -85,7 +85,7 @@
 String mountPoint = paths[1];
 if (root != null && cgroupPath != null) {
 if (root.equals("/")) {
-if (cgroupPath.equals("/")) {
+if (!cgroupPath.equals("/")) {
 finalPath = mountPoint + cgroupPath;
 } else {
 finalPath = mountPoint;
@@ -94,7 +94,7 @@
 if (root.equals(cgroupPath)) {
 finalPath = mountPoint;
 } else {
-if (root.indexOf(cgroupPath) == 0) {
+if (cgroupPath.indexOf(root) == 0) {
 if (cgroupPath.length() > root.length()) {
 String cgroupSubstr = 
cgroupPath.substring(root.length());
 finalPath = mountPoint + cgroupSubstr;
@@ -103,7 +103,7 @@
 }
 }
 }
-subSystemPaths.put(subSystem, new String[]{finalPath});
+subSystemPaths.put(subSystem, new String[]{finalPath, 
mountPoint});
 }
 }
 }


Bob.



> On Mar 22, 2019, at 6:43 AM, Severin Gehwolf  wrote:
> 
> Hi,
> 
> Please review this change which improves container detection support
> tin the JVM. While docker container detection works quite well the
> results for systemd slices with memory limits are mixed and depend on
> the Linux kernel version in use. With newer kernel versions becoming
> more widely used we should improve JVMs memory limit detection support
> as well. This should be entirely backwards compatible as the
> hierarchical limit will only be used if everything else is unlimited.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8217338
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217338/04/webrev/
> 
> Testing: Manual testing of -XshowSettings and -Xlog:os+container=trace
> with systemd slices on affected Linux distributions: Fedora 29,
> recent Ubuntu (18-10). Existing docker tests pass. I'm currently also
> running this through jdk/submit.
> 
> Thoughts?
> 
> Thanks,
> Severin
> 

RFR: 8220674: [TESTBUG] MetricsMemoryTester failcount test in docker container only works with debug JVMs

[Bob Vandette]

Please review this fix for a container test that fails on some Linux 
distributions.

The test fails to see the Memory Fail count metric value increase.  This is 
caused by
the fact that we are allowing ergonomics to select the amount of Heap size.  
This 
size varies depending on the amount of free memory that’s available on different
docker implementations.  

The fix is to force the VM to always set the heap size to the size of the 
containers
memory.  This change also stops allocating once it sees the fail count increase.
This is needed to keep the container from getting killed by the out of memory
killer.

The fix has been verified by the submitter along with a local run of the 
container tests.

BUG:

https://bugs.openjdk.java.net/browse/JDK-8220674

WEBREV:

http://cr.openjdk.java.net/~bobv/8220674/webrev/

Bob.

Re: RFR: 8220579: [Containers] SubSystem.java out of sync with osContainer_linux.cpp

[Bob Vandette]

The change looks good.  Thanks for fixing this.

I’d send this to core-libs (cc’d).

Bob.


> On Mar 14, 2019, at 12:51 PM, Severin Gehwolf  wrote:
> 
> Hi,
> 
> I'm not sure what the right list for Metrics.java[1] is. Assuming it's
> serviceability-dev:
> 
> Please review this one-liner for for SubSystem.java which currently
> behaves differently from the native implementation in
> osContainer_linux.cpp. Please see the details in the bug.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8220579
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8220579/01/webrev/
> 
> Testing:
> Manual testing of JDK-8217338 with Metrics.java support with/without
> this fix on Linux x86_64. Metrics tests and Docker tests continue to
> pass for fastdebug jvms (NOT for release jvms. see JDK-8220674, which
> was fun).
> 
> Thoughts?
> 
> Thanks,
> Severin
> 
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/641768acb12e/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
> 

Re: [PATCH] Windows 32-bit DLL name decoration

[Bob Vandette]

Hotspot has had support for decorated and non-decorated names for the JNI_OnLoad
functions.  Perhaps you should just follow the same procedure for the debug 
library.

#define JNI_ONLOAD_SYMBOLS   {"_JNI_OnLoad@8", "JNI_OnLoad"}
#define JNI_ONUNLOAD_SYMBOLS {"_JNI_OnUnload@8", "JNI_OnUnload"}
#define JVM_ONLOAD_SYMBOLS  {"_JVM_OnLoad@12", "JVM_OnLoad"}
#define AGENT_ONLOAD_SYMBOLS{"_Agent_OnLoad@12", "Agent_OnLoad"}
#define AGENT_ONUNLOAD_SYMBOLS  {"_Agent_OnUnload@4", "Agent_OnUnload"}
#define AGENT_ONATTACH_SYMBOLS  {"_Agent_OnAttach@12", “Agent_OnAttach”}

Bob.


> On Dec 11, 2018, at 11:43 AM, Simon Tooke  wrote:
> 
> On 2018-12-11 10:05 a.m., Alexey Ivanov wrote:
>> Hi everyone,
>> 
>> I came up with the following patch:
>> http://cr.openjdk.java.net/~aivanov/8214122/webrev.00/
>> 
>> It specifically addresses the problem in JDK-8214122 where on 32 bit
>> Windows jdwpTransport_OnLoad can exported with its plain and
>> __stdcall-mangled name. I used conditional compilation so that for
>> other platforms the code remains as it is now.
>> 
>> jshell starts successfully with this fix; an IDE debugger works as well.
>> 
> I am not a reviewer, but this patch only works for the specific case
> under discussion; the '@16' refers to the reserved stack size in the
> Pascal calling convention.  So, the patch only works for 16 bytes of
> parameters.  To be generic, the routine needs to have the stack size
> passed in by the caller.  If a generic fix isn't desired (and I hope it
> is), I'd prefer to see the caller simply pass the decorated or
> undecorated name depending on the Win32/64 defines.
> 
>#if defined(_WIN32) && !defined(_WIN64) onLoad =
>(jdwpTransport_OnLoad_t) dbgsysFindLibraryEntry(handle,
>"_jdwpTransport_OnLoad@16"); #else onLoad = (jdwpTransport_OnLoad_t)
>dbgsysFindLibraryEntry(handle, "jdwpTransport_OnLoad"); #endif
> 
> 
> Thanks,
> -Simon
> 
>> 
>> Regards,
>> Alexey
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8214122
>> 
>> On 10/12/2018 15:11, Magnus Ihse Bursie wrote:
 Since removing JNICALL is not an option, there are only two options:
 
 1. Add |/export| option to the Makefile or pragma-comment to the
 source file;
 2. Lookup the decorated name |_jdwpTransport_OnLoad@16| for Win32
 with fallback to undecorated one.
>>> Yes.
>>> 
>>> I think the correct solution here is 2.
>> 
> 

Re: RFR: 8206456 - [TESTBUG] docker jtreg tests fail on systems without cpuset.effective_cpus / cpuset.effective_mem

[Bob Vandette]

> On Jul 17, 2018, at 8:07 PM, mandy chung  wrote:
> 
> 
> 
> On 7/17/18 7:00 AM, Bob Vandette wrote:
>> Please review this fix which eliminates some docker/cgroup test failures 
>> when running on older
>> Linux kernels with missing cgroup metric files.
>> BUGS:
>> https://bugs.openjdk.java.net/browse/JDK-8206456
>> WEBREV:
>> http://cr.openjdk.java.net/~bobv/8206456/webrev/
> 
> Nit: It would be clearer to check for the specific metrics:
> 
> int[] cpusets = metrics.getEffectiveCpuSetCpus();
> if (cpusets.length != 0) {
>
> }
> 
> Same applies to getEffectiveCpuSetMems.  No need for a new webrev.

Thanks, I’ll do that cleanup.

> 
> Mandy
> P.S. I am not sure the conversion from the primitive to boxed type
> is necessary.  But this is not related to this issue.  You may
> want to take a look at that.

I’ll defer this issue to Harsha who wrote these tests since changing that is
out of scope for this fix.

Thanks,
Bob.

RFR: 8206456 - [TESTBUG] docker jtreg tests fail on systems without cpuset.effective_cpus / cpuset.effective_mem

[Bob Vandette]

Please review this fix which eliminates some docker/cgroup test failures when 
running on older
Linux kernels with missing cgroup metric files.

BUGS:
https://bugs.openjdk.java.net/browse/JDK-8206456

WEBREV:
http://cr.openjdk.java.net/~bobv/8206456/webrev/

This fix has been verified by the reporter of the issue.

Bob.

Re: RFR: 8205928 - [TESTBUG]: jdk/internal/platform/docker/TestDockerMemoryMetrics.java fails depending on kernel config

[Bob Vandette]

Matthais, who reported the issue, confirmed that this patch solves the problem.

Thanks,
Bob.

> On Jul 3, 2018, at 9:38 AM, Thomas Stüfe  wrote:
> 
> Hi Bob,
> 
> It does look fine from the outside. I did not test it though, since I
> have no suitable kernel.
> 
> Best Regards, Thomas
> 
> On Tue, Jul 3, 2018 at 3:13 PM, Bob Vandette  wrote:
>> Please review this small fix to correct a test failure when the Linux system 
>> kernel is
>> not configured with the CONFIG_MEMCG_KMEM option.
>> 
>> The Container Metric tests are dependent on docker which allow us to assume 
>> a certain minimum
>> Linux kernel configuration level. However, the kernel memory resource 
>> limiting feature is not a hard
>> requirement for docker. This test will need to be updated to allow for 
>> running on kernels without this
>> option.  A 0 return from the getKernelMemoryLimit is defined to indicate 
>> that this API is not available.
>> 
>> BUG: https://bugs.openjdk.java.net/browse/JDK-8205928
>> 
>> PROPOSED FIX:
>> 
>> diff --git a/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java 
>> b/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
>> --- a/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
>> +++ b/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
>> @@ -95,10 +95,11 @@
>> 
>> private static void testKernelMemoryLimit(String value) {
>> long limit = getMemoryValue(value);
>> -if (limit != Metrics.systemMetrics().getKernelMemoryLimit()) {
>> +long kmemlimit = Metrics.systemMetrics().getKernelMemoryLimit();
>> +if (kmemlimit != 0 && limit != kmemlimit) {
>> throw new RuntimeException("Kernel Memory limit not equal, 
>> expected : ["
>> + limit + "]" + ", got : ["
>> -+ Metrics.systemMetrics().getKernelMemoryLimit() + "]");
>> ++ kmemlimit + "]");
>> }
>> System.out.println("TEST PASSED!!!");
>> }

RFR: 8205928 - [TESTBUG]: jdk/internal/platform/docker/TestDockerMemoryMetrics.java fails depending on kernel config

[Bob Vandette]

Please review this small fix to correct a test failure when the Linux system 
kernel is
not configured with the CONFIG_MEMCG_KMEM option.

The Container Metric tests are dependent on docker which allow us to assume a 
certain minimum
Linux kernel configuration level. However, the kernel memory resource limiting 
feature is not a hard
requirement for docker. This test will need to be updated to allow for running 
on kernels without this
option.  A 0 return from the getKernelMemoryLimit is defined to indicate that 
this API is not available.

BUG: https://bugs.openjdk.java.net/browse/JDK-8205928

PROPOSED FIX:

diff --git a/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java 
b/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
--- a/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
+++ b/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java
@@ -95,10 +95,11 @@
 
 private static void testKernelMemoryLimit(String value) {
 long limit = getMemoryValue(value);
-if (limit != Metrics.systemMetrics().getKernelMemoryLimit()) {
+long kmemlimit = Metrics.systemMetrics().getKernelMemoryLimit();
+if (kmemlimit != 0 && limit != kmemlimit) {
 throw new RuntimeException("Kernel Memory limit not equal, 
expected : ["
 + limit + "]" + ", got : ["
-+ Metrics.systemMetrics().getKernelMemoryLimit() + "]");
++ kmemlimit + "]");
 }
 System.out.println("TEST PASSED!!!");
 }

Re: RFR(xxs): 8206243: java -XshowSettings fails if memory.limit_in_bytes overflows LONG.max

[Bob Vandette]

Looks ok.

Bob.

> On Jul 3, 2018, at 5:15 AM, Thomas Stüfe  wrote:
> 
> Thank you David!
> 
> I changed the webrev in place.
> 
> Thanks, Thomas
> 
> On Tue, Jul 3, 2018 at 10:37 AM, David Holmes  wrote:
>> Hi Thomas,
>> 
>> This seems okay.
>> 
>> Minor nit:
>> 
>> if(bigInt
>> 
>> Please add a space after 'if'
>> 
>> Thanks,
>> David
>> 
>> 
>> On 3/07/2018 6:20 PM, Thomas Stüfe wrote:
>>> 
>>> Hi all,
>>> 
>>> may I please have reviews for this small fix.
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8206243
>>> 
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8206243-java-xshowsettings-fails-for-large-values-of-memory-limit_in_bytes/webrev.00/webrev/
>>> 
>>> 
>>> On some Linux kernels, the unlimited value of memory.limit_in_bytes is
>>> returned as ULONG_MAX, not LONG_MAX.
>>> 
>>> - .../nightly $ cat //sys/fs/cgroup/memory/memory.limit_in_bytes
>>> 18446744073709551615
>>> 
>>> In those cases, java -XshowSettings will fail:
>>> 
>>> java -XshowSettings
>>> 
>>> Operating System Metrics:
>>> Provider: cgroupv1
>>> Effective CPU Count: 8
>>> CPU Period: 10us
>>> CPU Quota: -1
>>> CPU Shares: -1
>>> List of Processors, 8 total:
>>> 0 1 2 3 4 5 6 7
>>> List of Effective Processors, 0 total:
>>> List of Memory Nodes, 1 total:
>>> 0
>>> List of Available Memory Nodes, 0 total:
>>> CPUSet Memory Pressure Enabled: false
>>> Exception in thread "main" java.lang.NumberFormatException: For input
>>> string: "18446744073709551615"
>>> at
>>> java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
>>> at java.base/java.lang.Long.parseLong(Long.java:692)
>>> at java.base/java.lang.Long.parseLong(Long.java:817)
>>> at
>>> java.base/jdk.internal.platform.cgroupv1.SubSystem.getLongValue(SubSystem.java:106)
>>> at
>>> java.base/jdk.internal.platform.cgroupv1.Metrics.getMemoryLimit(Metrics.java:374)
>>> at
>>> java.base/sun.launcher.LauncherHelper.printSystemMetrics(LauncherHelper.java:385)
>>> 
>>> 
>>> Thank you,
>>> 
>>> Thomas
>>> 
>> 

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On Jun 12, 2018, at 1:43 AM, David Holmes  wrote:
> 
>> On 12/06/2018 3:31 PM, mandy chung wrote:
>> On 6/11/18 10:12 PM, David Holmes wrote:
>>> 
>>> 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.
>> I think the methods returning an array calls 
>> Subsystem::StringRangeToIntArray which returns an empty array.
>>  171 public static int[] StringRangeToIntArray(String range) {
>>  172 int[] ints = new int[0];
>>  173
>>  174 if (range == null) return ints;
> 
> I'm commenting on the specification of the Metrics interface:
> 
> http://cr.openjdk.java.net/~bobv/8203357/webrev.01/src/java.base/share/classes/jdk/internal/platform/Metrics.java.html
> 
> not any implementation.

Oh. I previously mentioned that I needed to correct the javadoc comments.  I 
had corrected the implementation but hadn’t fixed the
spec.  

Bob. 

> 
> Cheers,
> David
> 
>> Mandy

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On Jun 12, 2018, at 1:12 AM, David Holmes  wrote:
> 
> On 12/06/2018 9:30 AM, Bob Vandette wrote:
>>> On Jun 11, 2018, at 5:21 PM, David Holmes  wrote:
>>> 
>>> On 12/06/2018 12:12 AM, Bob Vandette wrote:
>>>>> On Jun 11, 2018, at 4:32 AM, David Holmes >>>> <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 >>>>>> <mailto:bob.vande...@oracle.com>> wrote:
>>>>

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On Jun 11, 2018, at 5:21 PM, David Holmes  wrote:
> 
> On 12/06/2018 12:12 AM, Bob Vandette wrote:
>>> On Jun 11, 2018, at 4:32 AM, David Holmes >> <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. 

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

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On Jun 11, 2018, at 4:07 AM, Robbin Ehn  wrote:
> 
> Hi Bob,
> 
> On 06/07/2018 07:43 PM, 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
> 
> Seems okay.
> 
> Metrics.java
> "Returns the length of the operating system time slice"
> 
> Note that is is only true if you are using a batch scheduler.
> Otherwise this period may be split on multiple 'time slices’.

This is a cgroup metric which uses CFS not the OS time slice.

 136 /**
 137  * Returns the length of the operating system time slice, in
 138  * milliseconds, for processes within the Isolation Group.

> 
> In printSystemMetrics there is no units, maybe intentional?

I’ll add ms for the quote/period output.  The memory metrics do have units.

> 
> Do we have support now in mach5 for docker jtreg, or do we still run these 
> separate?
> 
> You can ship it.

Thanks!
Bob.
> 
> Thanks for fixing, and super thanks for fixing the bug in PlainRead also!
> 
> /Robbin
> 
>> 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  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: 10
>>>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.
>>> 
>>> 

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On Jun 11, 2018, at 4:32 AM, David Holmes  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.

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

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  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: 10
>>>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.
>>> 
>>> 

Re: Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

I didn’t actually have any ERROR_MARGIN problems during testing.  I had issues 
with the testCpuConsumption  test in
http://cr.openjdk.java.net/~bobv/8203357/webrev.01/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java.html
 
<http://cr.openjdk.java.net/~bobv/8203357/webrev.01/test/lib/jdk/test/lib/containers/cgroup/MetricsTester.java.html>
I had to initialize the cpu usage values during setup rather than inside the 
test to ensure that sufficient cpu usage
had occurred by the time the test was run.   The original code executed and 
received the same values after attempting
to exec a linux utility.  My change uses the time taken to run several tests 
instead.  This seems to have eliminated any intermittent failures.

Bob.


> On Jun 8, 2018, at 12:30 AM, Harsha Wardhana B  
> wrote:
> 
> [Replying to all mailing-lists]
> Hi Misha,
> 
> The ERROR_MARGIN in tests was introduced to make the tests stable. There are 
> times where metric values (specifically CPU usage) can change drastically in 
> between two reads. The metrics value got from the API and the cgroup file can 
> be different and 0.1 ERROR_MARGIN should take care of that, though at times 
> even that may not be enough. Hence the CPU usage related tests only print a 
> warning if ERROR_MARGIN is exceeded.
> 
> Thanks
> Harsha
> 
> On Friday 08 June 2018 03:00 AM, Mikhailo Seledtsov wrote:
>> Hi Bob,
>> 
>>   I looked at the tests. In general they look good. I am a bit concerned 
>> about the use of ERROR_MARGIN in one of the tests. We need to make sure that 
>> the tests are stable, and do not produce intermittent failures.
>> 
>> 
>> Thank you,
>> Misha
>> 
>> On 6/7/18, 10: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  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: 10
>>>> 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
>>>> 

Ping!! Re: RFR: 8203357 Container Metrics

[Bob Vandette]

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  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: 10
>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.
> 
> 

Re: RFR: 8203357 Container Metrics

[Bob Vandette]

> On May 31, 2018, at 11:36 PM, mandy chung  wrote:
> 
> Hi Bob,
> 
> On 5/30/18 12:45 PM, Bob Vandette wrote:>
>> RFE: Container Metrics
>> https://bugs.openjdk.java.net/browse/JDK-8203357
>> WEBREV:
>> http://cr.openjdk.java.net/~bobv/8203357/webrev.00

Thanks for the review, here an updated webrev:

 http://cr.openjdk.java.net/~bobv/8203357/webrev.01/

> 
> Looks fine in general.  It's good to have this internal API ready
> for JFR and other library code to use.
> 
> I skimmed through the new tests.  It'd be good to add some comments
> to describe what it does (for example, set up a docker image etc).

DockerTestUtils.java does contain some comments describing what the
utility functions do.  I’ll add a brief comment in TestDockerCpuMetrics.java
and TestDockerMemoryMetrics.java explaining the test process.

> 
> launcher.properties
> 154 \-XshowSettings:system (Linux Only) show host system or container\n\
> 155 \  configuration and continue\n\
> 
> A newline can be placed after -XshowSettings:system consistent with
> other suboptions.

Done.  I also added a newline after the vm sub-option.

> 
> test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java
> 
> There are several long lines in the new test files such as:
>   MetricsCpuTester.java
>   MetricsMemoryTester.java
>   MetricsTester.java

> 
> It'd help future side-by-side review if they are wrapped. I think
> most of them are the construction of an exception.

Fixed.

> 
> I see a pattern of a name after @test and that is not strictly needed.
> 
> TestCgroupMetrics.java
>  25  * @test TestCgroupMetrics
> 
> TestDockerCpuMetrics.java
>  34  * @test TestSystemMetrics
> 
> TestDockerMemoryMetrics.java
>  30  * @test TestSystemMetrics
> 
> TestSystemMetrics.java
>  25  * @test TestSystemMetrics

Remove the names after @test.

> 
> This needs a CSR for the new -XshowSettings:system option.

I filed a CSR for this a few days ago.

https://bugs.openjdk.java.net/browse/JDK-8204107

Bob.



> 
> Mandy

RFR: 8203357 Container Metrics

[Bob Vandette]

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


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: 10
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.

Re: RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

> On May 24, 2018, at 2:42 PM, mandy chung <mandy.ch...@oracle.com> wrote:
> 
> 
> 
> On 5/23/18 7:39 AM, Bob Vandette wrote:
>>> Should this be an instance method?  like
>>> cpuacct.getLongValue("cpuacct.usage”);
> >
>> I did it this way in order to provide a centralized place to check
>> for missing subsystems.  The getLongValue method does the checking
>> for all subsystems
> 
> 137 if (subsystem == null) return 0L;
> 
> should this throw NPE?  same applies to all getXXXValue methods.
> 
> I think instance methods are appropriate since they obtain
> the stat for a given subsystem unless null subsystem can
> be passed as argument?

There will be situations where some platforms will not have all subsystems 
available.
I’ve documented the APIs to state that if a specific Metric is not available, 
the return
will be 0 (null).  This is implemented by passing the unavailable subsystem 
(NULL)
to the query methods. 

> 
>  73 public String Path() {
>  74 return path;
>  75 }
> 
> Just notice the method name "Path()" - should be lowercase "path()”?

Ok.

> 
>> Not sure what this is in reference to, please advise?
> 
>  51 private static final Metrics instance = initContainerSubSystems();
>  53 private static final String providerName = "cgroupv1";
> 
> INSTANCE and PROVIDER_NAME

Ok, thanks.  I’ll take care of that.

> 
>>> What does java --help-extra show?  The help message should include
>>> -XshowSettings:system only on Linux.
>> The message looks like it comes out of a resource file will need to
>> be localized. How do we make the message conditional on operating
>> system in that case? Can I just put (Linux Only) in the english
>> version and then get it localized?
> 
> The existing launcher.properties lists platform-specific options
> in text form:
> 
> The following options are Mac OS X specific:\n\
>-XstartOnFirstThread
>:
> 
> That's one possibility.

Yes, I saw that but wasn’t sure how new text that’s added to the 
launcher.properties file would get
localized.  Is there a process for getting this done?

Bob.

> 
>> Here’s the new output:
>> ./java -XshowSettings:system
> 
> Thanks for trimming the output.
> 
>> I’ll be sending out a webrev that includes the tests next week once
>> I’ve integrated them with my change and perform some testing on
>> different Linux systems and docker containers.
> 
> Sounds good.
> 
> Thanks
> Mandy

Re: RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

Hi Mandy,

I’m finally getting back to your review of this change now that we’ve made some 
progress on creating tests.

BTW: This Jira issue is now an RFE rather than a JEP 
(https://bugs.openjdk.java.net/browse/JDK-8203357 
<https://bugs.openjdk.java.net/browse/JDK-8203357>)

See comments below ...

> On Apr 17, 2018, at 10:25 PM, mandy chung <mandy.ch...@oracle.com> wrote:
> 
> 
> 
> On 4/3/18 10:09 PM, Bob Vandette wrote:
>> WEBREV:
>> 
>> http://cr.openjdk.java.net/~bobv/8182070/v01/webrev 
>> <http://cr.openjdk.java.net/~bobv/8182070/v01/webrev>
> 
> I reviewed the webrev and look okay in general. I will look through the 
> javadoc next.
> Metrics.java 
> 
>   37  * 1. All processes, including the current process within a 
> container.
> 
>includes the numbering. You can drop "1." and other numbers.
>  
>   42  * or
> 
> This adds a bullet.  Maybe dropping this line.
Done
> 
>   81  * @return The name of the provider or null if Metrics are 
>   82  * not enabled.
>   85 public String getProvider();
> 
> Should this method always return non-null name?
Done
> 
> For optional metric (when it's not available), the method returns 0.  For 
> example:
>  533  * @return The number of bytes transferred or 0 if this metric is 
> not available.
> 
> How does the client know if the metrics is not available or zero?  Or the 
> client does not care?
Unfortunately this is the way that cgroups works.  If the kernel has not been 
configured to provide the
metrics, the values are all 0’s.  I’m not sure what I can do about this 
behavior except document it as I have
done in the JavaDocs.
> 
> jdk/internal/platform/cgroupv1/Metrics.java
> 
>  274 return SubSystem.getLongValue(cpuacct, "cpuacct.usage");
> 
> Should this be an instance method?  like 
> cpuacct.getLongValue("cpuacct.usage”);
I did it this way in order to provide a centralized place to check for missing 
subsystems.  The getLongValue
method does the checking for all subsystems.

> 
> final field name can be made all caps.

Not sure what this is in reference to, please advise?
> 
> I know you are going to include regression tests.
> 
>> 
>> WEBREV including a Prototype MBEAN for exposing these Metrics:
>> 
>> This prototype will not be integrated as part of this JEP.  It’s for 
>> information only.
>> 
>> http://cr.openjdk.java.net/~bobv/8182070/v01/mbean-proto/ 
>> <http://cr.openjdk.java.net/~bobv/8182070/v01/mbean-proto/>
>> 
>> 
>> This feature adds a new -XshowSetting option “system” which displays the
>> available system Metrics.   
> 
> What does java --help-extra show?  The help message should include 
> -XshowSettings:system only on Linux.  

The message looks like it comes out of a resource file will need to be 
localized.
How do we make the message conditional on operating system in that case?  Can I 
just put
(Linux Only) in the english version and then get it localized?

> 
>> 
>> % java -XshowSettings:system
> 
> I expect this option shows static/configuration information rather than 
> timing statistics e.g. CPU time and usage.  It may be a smaller set but it 
> may be good information though.
> 
> It's more appropriate for monitoring tools to show the timing statistics and 
> resource consumption rather than the launcher.

I’ve removed any reporting of resource consumption APIs.  

Here’s the new output:

./java -XshowSettings:system
Operating System Metrics:
Provider: cgroupv1
Effective CPU Count: 24
CPU Period: 10
CPU Quota: -1
CPU Shares: -1
List of Processors, 24 total: 
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
List of Effective Processors, 24 total: 
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
List of Memory Nodes, 2 total: 
0 1 
List of Available Memory Nodes, 2 total: 
0 1 
CPUSet Memory Pressure Enabled: false
Memory Limit: Unlimited
Memory Soft Limit: Unlimited
Memory & Swap Limit: Unlimited
Kernel Memory Limit: Unlimited
TCP Memory Limit: 2048.00T
Out Of Memory Killer Enabled: true

I’ll be sending out a webrev that includes the tests next week once I’ve 
integrated them with my change and
perform some testing on different Linux systems and docker containers.

Thanks,
Bob.

> 
> Mandy
> 

RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

Here is a first pass at an implementation of the Container Awareness JEP.
This JEP adds an implementation of an internal API for the extraction of system 
metrics
for processes running in Isolation Groups (Containers).  The plan is to get the 
internal 
API integrated in JDK 11 with support for Linux x64 and then follow this work 
up with support
for alternate platforms, the addition of a JMX MBean and Java Flight Recorder.


JEP:

https://bugs.openjdk.java.net/browse/JDK-8182070


JAVADOC:

http://cr.openjdk.java.net/~bobv/8182070/v01/javadoc/jdk/internal/platform/Metrics.html


WEBREV:

http://cr.openjdk.java.net/~bobv/8182070/v01/webrev


WEBREV including a Prototype MBEAN for exposing these Metrics:

This prototype will not be integrated as part of this JEP.  It’s for 
information only.

http://cr.openjdk.java.net/~bobv/8182070/v01/mbean-proto/


This feature adds a new -XshowSetting option “system” which displays the
available system Metrics.

% java -XshowSettings:system

Operating System Metrics:
Provider: cgroupv1
Effective CPU Count: 24
CPUTime per Processor: 
[0]: 52805305 (ns)
[1]: 70799492 (ns)
[2]: 27449618 (ns)
[3]: 12957734 (ns)
[4]: 38382720 (ns)
[5]: 20325731 (ns)
[6]: 36374924 (ns)
[7]: 40279640 (ns)
[8]: 17557347 (ns)
[9]: 19056675 (ns)
[10]: 66185888 (ns)
[11]: 56539480 (ns)
[12]: 10009386 (ns)
[13]: 19139797 (ns)
[14]: 2257349 (ns)
[15]: 8712468 (ns)
[16]: 10306911 (ns)
[17]: 9814800 (ns)
[18]: 3516611 (ns)
[19]: 747174 (ns)
[20]: 4380756 (ns)
[21]: 11803118 (ns)
[22]: 1076297 (ns)
[23]: 8069315 (ns)

CPU Usage is: 550599580 (ns)
CPU User Usage is: 36 (ticks)
CPU System Usage is: 10 (ticks)
CPU Period: 10
CPU Quota: -1
CPU Shares: -1
CPU Number of Periods: 0
CPU Number of Throttled Periods: 0
CPU Throttled Time: 0
CPUSet Exclusive: false
CPUSet Memory Exclusive: false
List of Processors, 24 total: 
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
List of Effective Processors, 24 total: 
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
List of Memory Nodes, 2 total: 
0 1 
List of Available Memory Nodes, 2 total: 
0 1 
CPUSet Memory Pressure Enabled: false
CPUSet Memory Pressure: 0.0
Memory Failed Count: 0
Memory Limit: Unlimited
Memory Used: 43.31M
Max Memory Used: 48.82M
Memory Soft Limit: Unlimited
Memory & Swap Failed Count: 0.00K
Memory & Swap Limit: Unlimited
Memory & Swap Used: 43.93M
Max Memory & Swap Used: 48.82M
Kernel Memory Failed Count: 0.00K
Kernel Memory Limit: Unlimited
Kernel Memory Used: 0.00K
Kernel Max Memory Used: 0.00K
TCP Memory Failed Count: 0.00K
TCP Memory Limit: Unlimited
TCP Memory Used: 0.00K
TCP Max Memory Used: 0.00K
Out Of Memory Killer Enabled: true
BLKIO: Number of I/O Operations Completed: 42
BLKIO: Bytes Transferred from disk: 4923392

Bob Vandette

RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[Bob Vandette]

Please review this change that resolves the detection of Java processes that 
are running in cgroup
based containers.

This latest (and hopefully final) update of this fix addresses comments from 
David Holmes and Mandy Chung.  

Bug:

https://bugs.openjdk.java.net/browse/JDK-8193710

Webrev:

http://cr.openjdk.java.net/~bobv/8193710/webrev.02/

Summary:

This changeset enables the ability to use jcmd and jps running on a Host to
list the java processes that are running in docker (cgroup based) containers.

I’ve tested this change by examining processes running as root on both host and 
in
docker containers as well as under my userid using “jps and jcmd -l”.  
I’ve also tested updates to the getFile functions with a small example program 
that I wrote.


Here are some implementation details that I’ve added to the Linux specific 
implementation class:   

   
src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java

   /* Implementation Details:
*
* Java processes that run in docker containers are typically running
* under cgroups with separate pid namespaces which means that pids
* within the container are different that the pid which is visible
* from the host.  The container pids typically start with 1 and
* increase.  The java process running in the container will use these
* pids when creating the hsperfdata files.  In order to locate java
* processes that are running in containers, we take advantage of
* the Linux proc file system which maps the containers tmp directory
* to the hosts under /proc/{hostpid}/root/tmp.  We use the /proc status
* file /proc/{hostpid}/status to determine the containers pid and
* then access the hsperfdata file.  The status file contains an
* entry "NSPid:" which shows the mapping from the hostpid to the
* containers pid.
*
* Example:
*
* NSPid: 24345 11
*
* In this example process 24345 is visible from the host, 
* is running under the PID namespace and has a container specific
* pid of 11.
*
* The search for Java processes is done by first looking in the 
* traditional /tmp for host process hsperfdata files and then 
* the search will container in every /proc/*/root/tmp directory.  
* There are of course added complications to this search that 
* need to be taken into account.
*
* 1. duplication of tmp directories
*
* /proc/{hostpid}/root/tmp directories exist for many processes
* that are running on a Linux kernel that has cgroups enabled even
* if they are not running in a container.  To avoid this duplication,
* we compare the inode of the /proc tmp directories to /tmp and
* skip these duplicated directories.
*
* 2. Containerized processes without PID namespaces being enabled.
*
* If a container is running a Java process without namespaces being
* enabled, an hsperfdata file will only be located at
* /proc/{hostpid}/root/tmp/{hostpid}.  This is handled by
* checking the last component in the path for both the hostpid
* and potential namespacepids (if one exists).
*/

Bob.

Re: RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[Bob Vandette]

I put these new methods in VMSupport since this was the class containg the 
getVMTemporaryDirectory and
the intention of this class was document as:

/*
 * Support class used by JVMTI and VM attach mechanism.
 */
class VMSupport {

I could create a new PerfDataFileImpl or jdk.internal.jvmstat.VMSupport class 
with a Linux specific 
alternate implementation that contains these classes:
 
  getTemporaryDirectories
  getTemporaryDirectory
  getLocalVmId

But the getNamespaceVmId method is also needed by the attach mechanism.
I removed the duplicated functionality to avoid having to maintain this in
2 places.

Shouldn’t the attach functionality, in the future, be decoupled from jvmstat?  
It looked like
jcmd was using these jvmstat classes in order to avoid duplication but I don’t 
have all
the history.


Bob.



> On Jan 17, 2018, at 1:16 PM, mandy chung <mandy.ch...@oracle.com> wrote:
> 
> Hi Bob,
> 
> I think the new methods in VMSupport and probably with 
> VMSupport.getVMTemporaryDirectory should belong to jdk.internal.jvmstat as 
> they are specific for jvmstat and attach API to use.
> 
> W.r.t VMSupportImpl, the implementation is the same for all platforms except 
> Linux.  One option is to have a shared implementation class and instantiate 
> an alternate implementation class, if present, using Class::forName.
> Another minor point is that the new getVMTemporaryDirectories method can 
> return a List rather than an array.
> 
> Mandy
> 
> On 1/17/18 8:37 AM, Bob Vandette wrote:
>> Here’s an updated webrev addressing the comments from David Holmes.
>> 
>> 1. Moved new cgroup specific support from unix -> Linux and put stubs in all 
>> other OS's
>> 2. Reduced the size of the stack arrays in perfMemory_linux.cpp
>> 
>> 
>> There are still two outstanding issues:
>> 
>> [serviceability] :  Can we and should we remove the getFile methods in the 
>> PerfDataFile class?
>> 
>> [core-libs]:  Do you have any problems with the module info file changes 
>> impacting java.base?
>> 
>> Details:
>> 
>> Bug:
>> 
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8193710
>> 
>> 
>> Webrev:
>> 
>> 
>> http://cr.openjdk.java.net/~bobv/8193710/webrev.01/
>> 
>> 
>> Summary:
>> 
>> This changeset enables the ability to use jcmd and jps running on a Host to
>> list the java processes that are running in docker (cgroup based) containers.
>> 
>> I’ve tested this change by examining processes running as root on both host 
>> and in
>> docker containers as well as under my userid using “jps and jcmd -l”.  
>> I’ve also tested the getFile functions with a small example program that I 
>> wrote.
>> From what I can tell,  these two getFile functions are not used in the JDK 
>> sources
>> and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to 
>> have never worked!  
>> I’d really like to remove these two methods.
>> 
>> 140 candidate = new File(f.getName(), name);  <<——— This 
>> line drops the /tmp directory off of the path.
>> 
>> Here are some implementation details that I’ve added added to one of the 
>> Unix 
>> specific source files 
>> (src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)
>> 
>>/* Implementation Details:
>> *
>> * Java processes that run in docker containers are typically running
>> * under cgroups with separate pid namespaces which means that pids
>> * within the container are different that the pid which is visible
>> * from the host.  The container pids typically start with 1 and
>> * increase.  The java process running in the container will use these
>> * pids when creating the hsperfdata files.  In order to locate java
>> * processes that are running in containers, we take advantage of
>> * the Linux proc file system which maps the containers tmp directory
>> * to the hosts under /proc/{hostpid}/root/tmp.  We use the /proc status
>> * file /proc/{hostpid}/status to determine the containers pid and
>> * then access the hsperfdata file.  The status file contains an
>> * entry "NSPid:" which shows the mapping from the hostpid to the
>> * containers pid.
>> *
>> * Example:
>> *
>> * NSPid: 24345 11
>> *
>> * In this example process 24345 is visible from the host, 
>> * is running under the PID namespace and has a container specific
>> * pid of 11.
>> *
>> * The search for Java processes is done by first looking in the 
>>

RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[Bob Vandette]

Here’s an updated webrev addressing the comments from David Holmes.

1. Moved new cgroup specific support from unix -> Linux and put stubs in all 
other OS's
2. Reduced the size of the stack arrays in perfMemory_linux.cpp


There are still two outstanding issues:

[serviceability] :  Can we and should we remove the getFile methods in the 
PerfDataFile class?

[core-libs]:  Do you have any problems with the module info file changes 
impacting java.base?

Details:

Bug:

https://bugs.openjdk.java.net/browse/JDK-8193710

Webrev:

http://cr.openjdk.java.net/~bobv/8193710/webrev.01/

Summary:

This changeset enables the ability to use jcmd and jps running on a Host to
list the java processes that are running in docker (cgroup based) containers.

I’ve tested this change by examining processes running as root on both host and 
in
docker containers as well as under my userid using “jps and jcmd -l”.  
I’ve also tested the getFile functions with a small example program that I 
wrote.
From what I can tell,  these two getFile functions are not used in the JDK 
sources
and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to have 
never worked!  
I’d really like to remove these two methods.

140 candidate = new File(f.getName(), name);  <<——— This line 
drops the /tmp directory off of the path.

Here are some implementation details that I’ve added added to one of the Unix 
specific source files 
(src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)

   /* Implementation Details:
*
* Java processes that run in docker containers are typically running
* under cgroups with separate pid namespaces which means that pids
* within the container are different that the pid which is visible
* from the host.  The container pids typically start with 1 and
* increase.  The java process running in the container will use these
* pids when creating the hsperfdata files.  In order to locate java
* processes that are running in containers, we take advantage of
* the Linux proc file system which maps the containers tmp directory
* to the hosts under /proc/{hostpid}/root/tmp.  We use the /proc status
* file /proc/{hostpid}/status to determine the containers pid and
* then access the hsperfdata file.  The status file contains an
* entry "NSPid:" which shows the mapping from the hostpid to the
* containers pid.
*
* Example:
*
* NSPid: 24345 11
*
* In this example process 24345 is visible from the host, 
* is running under the PID namespace and has a container specific
* pid of 11.
*
* The search for Java processes is done by first looking in the 
* traditional /tmp for host process hsperfdata files and then 
* the search will container in every /proc/*/root/tmp directory.  
* There are of course added complications to this search that 
* need to be taken into account.
*
* 1. duplication of tmp directories
*
* /proc/{hostpid}/root/tmp directories exist for many processes
* that are running on a Linux kernel that has cgroups enabled even
* if they are not running in a container.  To avoid this duplication,
* we compare the inode of the /proc tmp directories to /tmp and
* skip these duplicated directories.
*
* 2. Containerized processes without PID namespaces being enabled.
*
* If a container is running a Java process without namespaces being
* enabled, an hsperfdata file will only be located at
* /proc/{hostpid}/root/tmp/{hostpid}.  This is handled by
* checking the last component in the path for both the hostpid
* and potential namespacepids (if one exists).
*/



Bob.

Re: RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[Bob Vandette]

> On Jan 11, 2018, at 9:44 PM, David Holmes <david.hol...@oracle.com> wrote:
> 
> Hi Bob,
> 
> Some initial discussion and commentary. I'm a bit concerned by how much 
> accommodating docker penalises code that is not using containers.

True but querying the number of Java processes is at least not in a performance 
critical area.

> 
> On 12/01/2018 4:41 AM, Bob Vandette wrote:
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8193710
>> Webrev:
>> http://cr.openjdk.java.net/~bobv/8193710/webrev.00/
>> Summary:
>> This changeset enables the ability to use jcmd and jps running on a Host to
>> list the java processes that are running in docker (cgroup based) containers.
> 
> Can you clarify this for me please. I assume we are still only listing 
> processes for the current user - correct?

No, it will report any Java processes that the current user has permission to 
see (hsperfdata* files are readable).
If you run as root, you see all processes.

> So the issue is if I am running mutiple JVMs and also running a Docker 
> process that itself contains JVMs then jps outside of Docker won't show the 
> JVMs that are inside Docker.

Yes, this is what is being fixed.

> And presumably the opposite is also true?
> 

Yes, this is also true but is not being fixed by this change.  I am only fixing 
the case where you run jps on a host and
want to see all java processes running (in and out of containers).


> If we change jps to find all JVMs on the host for the current user, whether 
> in a container or not (and whether jps is run from in a container or not?) 
> are the process id's that get returned directly useable from where jps was 
> run?

Given the above constraints, yes.

The results jps can passed to jcmd to query things like VM.version

> 
>> I’ve tested this change by examining processes running as root on both host 
>> and in
>> docker containers as well as under my userid using “jps and jcmd -l”.
>> I’ve also tested the getFile functions with a small example program that I 
>> wrote.
>> From what I can tell,  these two getFile functions are not used in the JDK 
>> sources
>> and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to 
>> have never worked!
>> I’d really like to remove these two methods.
> 
> If they are not used by anything then by all means drop them.
I saw some Javadocs on the web documenting these methods leading me to believe 
that we would need
to deprecate these even though they are internal APIs.  I’ll ask core-libs.

http://openjdk.java.net/groups/serviceability/jvmstat/sun/jvmstat/perfdata/monitor/protocol/local/PerfDataFile.html#getFile(int)

> 
>>  140 candidate = new File(f.getName(), name);  <<——— This 
>> line drops the /tmp directory off of the path.
>> Here are some implementation details that I’ve added added to one of the Unix
>> specific source files 
>> (src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)
> 
> I have an issue with this. This is not "unix" (ie anything other than 
> windows) support code, it is Linux support code. I'm also unclear why this 
> needs to be lifted out of the PerfMemory subsystem.
You are correct, I’ll move this into a linux specific directory.  I factored 
these functions in order to isolate the cgroup specific functionality
in an OS specific tree.

> 
> And if you're going to mess with the jdk.internal module then I think 
> core-libs folk will want to know before hand.

Will do.

> 
> Other comments ...
> 
> src/hotspot/os/linux/perfMemory_linux.cpp
> 
> I'm concerned about the MAXPATHLEN stack buffers and the extra overhead being 
> added to the normal non-container path.

I believe the impacted paths are only used when attaching to the VM and should 
not impact normal startup.

> We've had issues in the past where a newly added stack buffer caused 
> StackOverflowError. We don't need a 4K buffer because 
> os::get_temp_directory() just returns "/tmp" and we're dealing with all Linux 
> code here. I don't like making assumptions like this but ...
> 
> #define TMP_BUFFER_LEN (4 + 21)
> static char* get_user_tmp_dir(const char* user, int vmid, int nspid) {
>  char buffer[TMP_BUFFER_LEN];
>  char* tmpdir = os::get_temp_directory();
>  assert(strlen(tmpdir) == 4, "No longer using /tmp - update buffer size");
>  if (nspid != -1) {
>jio_snprintf(buffer, TMP_BUFFER_LEN, "/proc/%d/root%s", vmid, tmpdir);
>tmpdir = buffer;
>  }
>  ...
> 
> 
> 657   char fname[64];
> 
> Maximum length of "/proc/%d/status" is 23.

Ok, I’ll reduce the buffer sizes I use.

Thanks,
Bob.


> 
> Thanks,
> David

RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

[Bob Vandette]

Bug:

https://bugs.openjdk.java.net/browse/JDK-8193710

Webrev:

http://cr.openjdk.java.net/~bobv/8193710/webrev.00/

Summary:

This changeset enables the ability to use jcmd and jps running on a Host to
list the java processes that are running in docker (cgroup based) containers.

I’ve tested this change by examining processes running as root on both host and 
in
docker containers as well as under my userid using “jps and jcmd -l”.  
I’ve also tested the getFile functions with a small example program that I 
wrote.
From what I can tell,  these two getFile functions are not used in the JDK 
sources
and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to have 
never worked!  
I’d really like to remove these two methods.

 140 candidate = new File(f.getName(), name);  <<——— This line 
drops the /tmp directory off of the path.

Here are some implementation details that I’ve added added to one of the Unix 
specific source files 
(src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)

/* Implementation Details:
 *
 * Java processes that run in docker containers are typically running
 * under cgroups with separate pid namespaces which means that pids
 * within the container are different that the pid which is visible
 * from the host.  The container pids typically start with 1 and
 * increase.  The java process running in the container will use these
 * pids when creating the hsperfdata files.  In order to locate java
 * processes that are running in containers, we take advantage of
 * the Linux proc file system which maps the containers tmp directory
 * to the hosts under /proc/{hostpid}/root/tmp.  We use the /proc status
 * file /proc/{hostpid}/status to determine the containers pid and
 * then access the hsperfdata file.  The status file contains an
 * entry "NSPid:" which shows the mapping from the hostpid to the
 * containers pid.
 *
 * Example:
 *
 * NSPid: 24345 11
 *
 * In this example process 24345 is visible from the host, 
 * is running under the PID namespace and has a container specific
 * pid of 11.
 *
 * The search for Java processes is done by first looking in the 
 * traditional /tmp for host process hsperfdata files and then 
 * the search will container in every /proc/*/root/tmp directory.  
 * There are of course added complications to this search that 
 * need to be taken into account.
 *
 * 1. duplication of tmp directories
 *
 * /proc/{hostpid}/root/tmp directories exist for many processes
 * that are running on a Linux kernel that has cgroups enabled even
 * if they are not running in a container.  To avoid this duplication,
 * we compare the inode of the /proc tmp directories to /tmp and
 * skip these duplicated directories.
 *
 * 2. Containerized processes without PID namespaces being enabled.
 *
 * If a container is running a Java process without namespaces being
 * enabled, an hsperfdata file will only be located at
 * /proc/{hostpid}/root/tmp/{hostpid}.  This is handled by
 * checking the last component in the path for both the hostpid
 * and potential namespacepids (if one exists).
 */

Bob.

Re: RFR - Changes to address CCC 8014135: Support for statically linked agents

[Bob Vandette]

On Aug 2, 2013, at 11:11 PM, Bill Pittore wrote:

 On 8/2/2013 9:12 PM, serguei.spit...@oracle.com wrote:
 Hi Bill,
 
 A couple of more questions.
 
 webrev.01/jvmti.html
 
 An agent L whose image has been combined with the VM *is defined* as 
 /statically linked/
 if and only if the agent exports a function called Agent_OnLoad_L.
 
 A question to the above.
 Are we going to allow to link a library dynamically if it exports both
 the Agent_OnLoad and Agent_OnLoad_L functions?
 It can be convenient if a library exports both Agent_OnLoad and 
 Agent_OnLoad_L
 as it can be linked statically or dynamically depending on the need without 
 changes.
 
 It would be nice but the problem is that you could only link one agent into 
 the VM if it exported Agent_OnLoad. Otherwise there would be a symbol 
 collision with the second agent you linked in that also had Agent_OnLoad. As 
 an agent developer you will have to select one or the other at build time, 
 either statically linked in or dynamic.
 You already added the following statement to the JVMTI spec:
 If a /statically linked/ agent L exports a function called Agent_OnLoad_L and
  a function called Agent_OnLoad, the Agent_OnLoad function will be ignored.
 
 Could we say it in a shorter form?:
 If a /statically linked/ agent L exports a function called Agent_OnLoad,
  the Agent_OnLoad function will be ignored.
 I believe I copied this from JNI static linking JEP.  If so, I'll probably 
 leave it as is just for consistency with JNI static spec. JVM TI static 
 linking spec is closely related to JNI static linking spec.
 
 In this context would it be reasonable to add another statement:
 If a /dynamically linked/ agent L exports a function called Agent_OnLoad_L,
  the Agent_OnLoad_L function will be ignored.
 

I'd rather leave this undefined since the behavior is very platform specific. 
The way we determine if a library is statically linked is by the presence of 
the Agent_OnLoad_L function.  
If this function exists in a dynamically linked library, it will be treated as 
a static library if by some chance 
it's attempted to be loaded twice, since the symbol will may be visible in the 
running process.

Bob.


 The same questions apply to the Agent_OnAttach and Agent_OnAttach_L entry 
 points.
 
 I'm out on vacation for a couple of weeks so I'll leave it up to Bob V. and 
 yourself if you guys want to hash out better/different wording.
 
 bill
 
 Thanks,
 Serguei
 
 
 On 7/30/13 12:17 PM, bill.pitt...@oracle.com wrote:
 Thanks Serguei for the comments. Some comments inline. I updated the 
 webrevs at
 http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.02/
 http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.01/
 
 http://cr.openjdk.java.net/~bpittore/8014135/javadoc/index.html
 http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.01/jvmti.html
 
 
 On 7/26/2013 5:00 AM, serguei.spit...@oracle.com wrote:
 Hi Bill,
 
 Sorry for the big delay.
 
 
 http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.01/
 
 
 src/share/classes/com/sun/tools/attach/VirtualMachine.java:
 
 
 I'm suggesting to use the reference *codeAgent_OnAttach[_L]/code**||* 
 even more consistently.
 (If, in some cases, you prefer the longer form to underline the difference 
 between
 dynamically and statically linked libraries then feel free to leave it as 
 it is.)
 
 It would simplify the following fragments:
 
 304  * It then causes the target VM to invoke the 
 codeAgent_OnAttach/code function
 305  * or, for a statically linked agent named 'L', the 
 codeAgent_OnAttach_L/code function
   ==
 
 304  * It then causes the target VM to invoke the 
 codeAgent_OnAttach[_L]/code function
 
 409  * It then causes the target VM to invoke the 
 codeAgent_OnAttach/code
 410  * function or, for a statically linked agent named 'L', the
 411  * codeAgent_OnAttach_L/code function as specified in the
  ==
  409  * It then causes the target VM to invoke the 
 codeAgent_OnAttach[_L]/code
  410  * function as specified in the
 
 I left the above as is since it's part of the method description. The 
 following fragments below I simplified as you suggested.
 
 
 the following 4 identical fragments:
 
  341  *  If the codeAgent_OnAttach/code function returns 
 an error
  342  *  or, for a statically linked agent named 'L', if the
  343  *  codeAgent_OnAttach_L/code function returns
  344  *  an error.
  375  *  If the codeAgent_OnAttach/code function returns 
 an error
  376  *  or, for a statically linked agent named 'L', if the
  377  *  codeAgent_OnAttach_L/code function returns
  378  *  an error.
  442  *  If the codeAgent_OnAttach/code function returns 
 an error
  443  *  or, for a statically linked agent named 'L', if the
  444  *  codeAgent_OnAttach_L/code function returns an 
 error
  475  *  If the 

Re: RFR - Changes to address CCC 8014135: Support for statically linked agents

[Bob Vandette]

Serguei, 

Are you ok with the webrev at this point or are you waiting for any changes 
from Bill?

I've asked Coleen to review the code since she's an official Reviewer but she'd 
like to make
sure the serviceability team is ok with the changes.

Bob.


On Aug 3, 2013, at 12:34 AM, serguei.spit...@oracle.com wrote:

 On 8/2/13 8:11 PM, Bill Pittore wrote:
 On 8/2/2013 9:12 PM, serguei.spit...@oracle.com wrote:
 Hi Bill,
 
 A couple of more questions.
 
 webrev.01/jvmti.html
 
 An agent L whose image has been combined with the VM *is defined* as 
 /statically linked/
 if and only if the agent exports a function called Agent_OnLoad_L.
 
 A question to the above.
 Are we going to allow to link a library dynamically if it exports both
 the Agent_OnLoad and Agent_OnLoad_L functions?
 It can be convenient if a library exports both Agent_OnLoad and 
 Agent_OnLoad_L
 as it can be linked statically or dynamically depending on the need without 
 changes.
 
 It would be nice but the problem is that you could only link one agent into 
 the VM if it exported Agent_OnLoad. Otherwise there would be a symbol 
 collision with the second agent you linked in that also had Agent_OnLoad. As 
 an agent developer you will have to select one or the other at build time, 
 either statically linked in or dynamic.
 
 I did not want to use the Agent_OnLoad for statically linked agent.
 Just wanted to say that the presence of the Agent_OnLoad_L must be ignored
 if the agent is linked dynamically.
 Maybe this rule needs to be clearly stated in the JVMTI spec.
 
 You already added the following statement to the JVMTI spec:
 If a /statically linked/ agent L exports a function called Agent_OnLoad_L 
 and
  a function called Agent_OnLoad, the Agent_OnLoad function will be ignored.
 
 Could we say it in a shorter form?:
 If a /statically linked/ agent L exports a function called Agent_OnLoad,
  the Agent_OnLoad function will be ignored.
 I believe I copied this from JNI static linking JEP.  If so, I'll probably 
 leave it as is just for consistency with JNI static spec. JVM TI static 
 linking spec is closely related to JNI static linking spec.
 
 I see. Then it is Ok with me.
 
 
 In this context would it be reasonable to add another statement:
 If a /dynamically linked/ agent L exports a function called Agent_OnLoad_L,
  the Agent_OnLoad_L function will be ignored.
 
 The same questions apply to the Agent_OnAttach and Agent_OnAttach_L entry 
 points.
 
 I'm out on vacation for a couple of weeks so I'll leave it up to Bob V. and 
 yourself if you guys want to hash out better/different wording.
 
 Thank you for the quick reply, and have a nice vacation!
 
 Thanks,
 Serguei
 
 
 bill
 
 Thanks,
 Serguei
 
 
 On 7/30/13 12:17 PM, bill.pitt...@oracle.com wrote:
 Thanks Serguei for the comments. Some comments inline. I updated the 
 webrevs at
 http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.02/
 http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.01/
 
 http://cr.openjdk.java.net/~bpittore/8014135/javadoc/index.html
 http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.01/jvmti.html
 
 
 On 7/26/2013 5:00 AM, serguei.spit...@oracle.com wrote:
 Hi Bill,
 
 Sorry for the big delay.
 
 
 http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.01/
 
 
 src/share/classes/com/sun/tools/attach/VirtualMachine.java:
 
 
 I'm suggesting to use the reference *codeAgent_OnAttach[_L]/code**||* 
 even more consistently.
 (If, in some cases, you prefer the longer form to underline the 
 difference between
 dynamically and statically linked libraries then feel free to leave it as 
 it is.)
 
 It would simplify the following fragments:
 
 304  * It then causes the target VM to invoke the 
 codeAgent_OnAttach/code function
 305  * or, for a statically linked agent named 'L', the 
 codeAgent_OnAttach_L/code function
   ==
 
 304  * It then causes the target VM to invoke the 
 codeAgent_OnAttach[_L]/code function
 
 409  * It then causes the target VM to invoke the 
 codeAgent_OnAttach/code
 410  * function or, for a statically linked agent named 'L', the
 411  * codeAgent_OnAttach_L/code function as specified in the
  ==
  409  * It then causes the target VM to invoke the 
 codeAgent_OnAttach[_L]/code
  410  * function as specified in the
 
 I left the above as is since it's part of the method description. The 
 following fragments below I simplified as you suggested.
 
 
 the following 4 identical fragments:
 
  341  *  If the codeAgent_OnAttach/code function returns 
 an error
  342  *  or, for a statically linked agent named 'L', if the
  343  *  codeAgent_OnAttach_L/code function returns
  344  *  an error.
  375  *  If the codeAgent_OnAttach/code function returns 
 an error
  376  *  or, for a statically linked agent named 'L', if the
  377  *  codeAgent_OnAttach_L/code function returns
  378  *  an error.
  

Re: RFR - Changes to address CCC 8014135: Support for statically linked agents

[Bob Vandette]

Thanks for the suggestion Jeremy but the JVMTI agent change is being 
implemented as part of the
original JNI static library implementation where we explicitly prohibited 
dynamic libraries to be loaded
if a static library of the same name is baked into the launcher.  

If a library L is statically linked then it will be prohibited to link a 
library of the same name dynamically.

The primary motivation for requiring this for the -agentpath as well as the 
java.lang.System.load API is to
minimize changes to java source code.  You can statically or dynamically link a 
library without changing
any of the code that attempts to use the library.  The only thing that changes 
is the OnLoad name in the library
and the link options.

If you'd like to optionally use different implementations, you can end up with 
the same behavior by using a 
System property which alters the String of the name of the library.  

Bob.


On Jul 10, 2013, at 3:29 PM, BILL PITTORE wrote:

 On 7/3/2013 6:32 PM, Jeremy Manson wrote:
 I know that this is mentioned in the JEP, but does it really make sense to 
 have -agentpath work here, rather than just -agentlib?  I would think that 
 specifying a full pathname and getting the library loaded out of the 
 launcher would be a little surprising to people who are basically saying 
 please load this agent from the given path.
 
 Also, in practice, we do something very similar at Google, and, when 
 testing, I find it occasionally useful to be able to say please load this 
 agent on the command line via the agentpath I gave you, rather than loading 
 the one with the same name I baked into the launcher.
 
 (FWIW, when we did this internally at Google, we added a special -XX flag 
 for when the agent is in the launcher.  I know that you don't want to go 
 that way, though.)
 
 Thanks for the comments.  I would say the thinking here is that we want this 
 to be as transparent as possible to the end user. In our view of the end user 
 is someone perhaps using an IDE and the actual execution of the VM with agent 
 arguments is hidden. In your case it sounds like you are actually developing 
 agents which is a whole different ball game. I could certainly see adding a 
 -XX argument that forces agentpath to load the external library which would 
 be good for agent development and debugging. No need to relink the entire VM 
 with the new agent. Our tech lead is out on vacation this week so I'll bring 
 this up when he's back.
 
 bill
 
 
 On Wed, Jul 3, 2013 at 2:17 PM, BILL PITTORE bill.pitt...@oracle.com 
 mailto:bill.pitt...@oracle.com wrote:
 
These changes address bug 8014135 which adds support for
statically linked agents in the VM. This is a followup to the
recent JNI spec changes that addressed statically linked JNI
libraries( 8005716).
The JEP for this change is the same JEP as the JNI changes:
http://openjdk.java.net/jeps/178
 
Webrevs are here:
 
http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.00/
http://cr.openjdk.java.net/%7Ebpittore/8014135/jdk/webrev.00/
http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/
http://cr.openjdk.java.net/%7Ebpittore/8014135/hotspot/webrev.00/
 
The changes to jvmti.xml can also be seen in the output file that
I placed here:
http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/jvmti.html

 http://cr.openjdk.java.net/%7Ebpittore/8014135/hotspot/webrev.00/jvmti.html
 
Thanks,
bill
 
 
 
 

Re: review request for JDK-8013461 There is a symbol AsyncGetCallTrace in libjvm.symbols that does not exist in minimal/libjvm.a when DEBUG_LEVEL == release

[Bob Vandette]

Joe,

I'm ok with this approach.

Bob.

On May 22, 2013, at 3:27 PM, Joseph Provino wrote:

 Is there a consensus what is in the webrev is okay?
 
 The change is to include forte.cpp in the minimal jvm but to
 conditionalize the code so that only AsyncGetCallTrace()
 is defined with the minimal jvm.
 
 Webrev is here: http://cr.openjdk.java.net/~jprovino/8013461/webrev.01
 
JDK-8013461 https://jbs.oracle.com/bugs/browse/JDK-8013461
There is a symbol AsyncGetCallTrace in libjvm.symbols that does not exist 
 in
minimal/libjvm.a when DEBUG_LEVEL == release
 
 joe
 
 On 05/21/2013 04:52 PM, JOSEPH PROVINO wrote:
 
 On 5/21/2013 4:00 PM, Oleg Mazurov wrote:
 Though formally not part of the Solaris Studio team any more here is my 
 opinion based on my recollection of how I implemented interaction with the 
 JVM via AsyncGetCallTrace.
 It's looked up using dlsym. If the symbol is not there Java callstack 
 collection is shut down. I understand in your case even JVMTI is not there 
 so the dlsym call will not be made.
 From that perspective there is no difference whether the symbol is present 
 and returns an error code or not present at all.
 
 Oleg, then it sounds like what we have will work.
 
 Thanks for the quick reply.
 
 joe
 
 
-- Oleg
 
 On 5/21/2013 12:19 PM, JOSEPH PROVINO wrote:
 
 On 5/21/2013 3:16 PM, serguei.spit...@oracle.com wrote:
 On 5/21/13 11:26 AM, JOSEPH PROVINO wrote:
 
 On 5/21/2013 2:23 PM, Staffan Larsen wrote:
 On 21 maj 2013, at 17:35, JOSEPH PROVINO joseph.prov...@oracle.com 
 wrote:
 
 On 5/21/2013 3:06 AM, David Holmes wrote:
 Hi Staffan,
 
 On 21/05/2013 4:49 PM, Staffan Larsen wrote:
 On 21 maj 2013, at 04:34, David Holmes david.hol...@oracle.com 
 wrote:
 
 added servicability
 
 Hi Joe,
 
 As I have previously stated you copied the struct definitions 
 instead of moving them outside the ifdef.
 
 Serviceability folk: we are particularly interested in whether the 
 use of ticks_no_class_load is deemed appropriate in this situation. 
 Who will be consuming this value?
 Since you have opted for the simple fix of having an exported but 
 non-functional AsyncGetCallTrace instead of actually removing the 
 symbol from the symbol files (which is the proposed solution in the 
 bug report),
 That would be a simpler solution semantically but the only way I can 
 see to do that is to use a text replacement mechanism in the build 
 files - as is done for the dynamic vtable symbols. I find that less 
 appealing than simply exporting an interface that is configured to 
 report an error (which is essentially what all the optional 
 interfaces do under the minimal VM).
 
 I would like you to include a comment about this in the source. 
 Right now it's very unclear why there is an exported function that 
 only returns an error.
 
 As to the appropriate return value, I don't know. The only caller 
 should be the Sun Studio profiler,
 Does anyone know where to find instructions on how to run the collector 
 which would get the error return value?
  and I'm not sure how it will handle this case if ever run. The 
 possible return values aren't very well documented.
 I guess we need to try and run it to find out.
 Okay, do either of you feel strongly about how this should be fixed -- 
 return an error or remove the symbol?
 No, I don't feel strongly either way, but a comment in the code would 
 be nice.
 How much effort should I put into finding out what Sun Studio profiler 
 does when it gets -1?
 
 Let's ask the Solaris Studio guys directly.
 I'm adding Oleg to the mailing list.
 
 Oleg,
 
 Could you, please, share your view on this problem?
 
 In particular what will the Sun Studio Profiler collector do if it gets 
 the error
 
 trace-num_frames = ticks_no_class_load; // -1
 
 Thanks.
 
 joe
 
 
 
 Thanks,
 Serguei
 
 
 
 
 
 joe
 
 
 Thanks,
 /Staffan
 
 
 joe
 
 Thanks,
 David
 
 /Staffan
 
 Thanks,
 David
 
 On 21/05/2013 5:10 AM, JOSEPH PROVINO wrote:
 The change is to include forte.cpp in the minimal jvm but to
 conditionalize the code so that
 only AsyncGetCallTrace() is defined with the minimal jvm.
 
 Webrev is here: 
 http://cr.openjdk.java.net/~jprovino/8013461/webrev.00/
 
  * JDK-8013461 
 https://jbs.oracle.com/bugs/browse/JDK-8013461There is
a symbol AsyncGetCallTrace in libjvm.symbols that does not 
 exist in
minimal/libjvm.a when DEBUG_LEVEL == release
 https://jbs.oracle.com/bugs/browse/JDK-8013461
 
 Thanks.
 
 joe
 
 
 
 
 
 
 
 

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[bob . vandette]

Changeset: b92c45f2bc75
Author:bobv
Date:  2011-02-02 11:35 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/b92c45f2bc75

7016023: Enable building ARM and PPC from src/closed repository
Reviewed-by: dholmes, bdelsart

! make/Makefile
+ make/closed.make
! make/jprt.properties
! make/linux/Makefile
! make/linux/makefiles/adlc.make
+ make/linux/makefiles/arm.make
! make/linux/makefiles/buildtree.make
+ make/linux/makefiles/ppc.make
! make/linux/makefiles/rules.make
! make/linux/makefiles/top.make
! make/linux/makefiles/vm.make
+ make/linux/platform_arm
+ make/linux/platform_ppc
! src/os/linux/vm/osThread_linux.cpp
! src/os/linux/vm/os_linux.cpp
! src/os/linux/vm/os_linux.inline.hpp
! src/os/linux/vm/thread_linux.inline.hpp
! src/share/vm/asm/assembler.cpp
! src/share/vm/asm/assembler.hpp
! src/share/vm/asm/codeBuffer.hpp
! src/share/vm/c1/c1_Defs.hpp
! src/share/vm/c1/c1_FpuStackSim.hpp
! src/share/vm/c1/c1_FrameMap.cpp
! src/share/vm/c1/c1_FrameMap.hpp
! src/share/vm/c1/c1_Instruction.hpp
! src/share/vm/c1/c1_LIRAssembler.cpp
! src/share/vm/c1/c1_LIRAssembler.hpp
! src/share/vm/c1/c1_LinearScan.cpp
! src/share/vm/c1/c1_LinearScan.hpp
! src/share/vm/c1/c1_MacroAssembler.hpp
! src/share/vm/c1/c1_globals.hpp
! src/share/vm/classfile/classFileStream.hpp
! src/share/vm/classfile/stackMapTable.hpp
! src/share/vm/classfile/verifier.cpp
! src/share/vm/code/codeBlob.cpp
! src/share/vm/code/compiledIC.hpp
! src/share/vm/code/icBuffer.cpp
! src/share/vm/code/relocInfo.cpp
! src/share/vm/code/relocInfo.hpp
! src/share/vm/code/vmreg.hpp
! src/share/vm/compiler/disassembler.cpp
! src/share/vm/compiler/disassembler.hpp
! src/share/vm/interpreter/abstractInterpreter.hpp
! src/share/vm/interpreter/bytecode.hpp
! src/share/vm/interpreter/bytecodeInterpreter.cpp
! src/share/vm/interpreter/bytecodeInterpreter.hpp
! src/share/vm/interpreter/bytecodeInterpreter.inline.hpp
! src/share/vm/interpreter/bytecodeStream.hpp
! src/share/vm/interpreter/bytecodes.cpp
! src/share/vm/interpreter/bytecodes.hpp
! src/share/vm/interpreter/cppInterpreter.hpp
! src/share/vm/interpreter/cppInterpreterGenerator.hpp
! src/share/vm/interpreter/interpreter.hpp
! src/share/vm/interpreter/interpreterGenerator.hpp
! src/share/vm/interpreter/interpreterRuntime.cpp
! src/share/vm/interpreter/interpreterRuntime.hpp
! src/share/vm/interpreter/templateInterpreter.hpp
! src/share/vm/interpreter/templateInterpreterGenerator.hpp
! src/share/vm/interpreter/templateTable.hpp
! src/share/vm/oops/constantPoolOop.hpp
! src/share/vm/oops/oop.inline.hpp
! src/share/vm/oops/typeArrayOop.hpp
! src/share/vm/opto/buildOopMap.cpp
! src/share/vm/opto/c2_globals.hpp
! src/share/vm/opto/c2compiler.cpp
! src/share/vm/opto/compile.cpp
! src/share/vm/opto/gcm.cpp
! src/share/vm/opto/locknode.hpp
! src/share/vm/opto/output.hpp
! src/share/vm/opto/regmask.cpp
! src/share/vm/opto/regmask.hpp
! src/share/vm/opto/runtime.cpp
! src/share/vm/prims/jniCheck.cpp
! src/share/vm/prims/jni_md.h
! src/share/vm/prims/jvmtiClassFileReconstituter.cpp
! src/share/vm/runtime/deoptimization.cpp
! src/share/vm/runtime/dtraceJSDT.hpp
! src/share/vm/runtime/frame.cpp
! src/share/vm/runtime/frame.hpp
! src/share/vm/runtime/frame.inline.hpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/icache.hpp
! src/share/vm/runtime/java.cpp
! src/share/vm/runtime/javaCalls.hpp
! src/share/vm/runtime/javaFrameAnchor.hpp
! src/share/vm/runtime/os.hpp
! src/share/vm/runtime/registerMap.hpp
! src/share/vm/runtime/relocator.hpp
! src/share/vm/runtime/safepoint.cpp
! src/share/vm/runtime/sharedRuntime.cpp
! src/share/vm/runtime/stackValueCollection.cpp
! src/share/vm/runtime/statSampler.cpp
! src/share/vm/runtime/stubCodeGenerator.cpp
! src/share/vm/runtime/stubRoutines.hpp
! src/share/vm/runtime/thread.hpp
! src/share/vm/runtime/threadLocalStorage.hpp
! src/share/vm/runtime/vmStructs.cpp
! src/share/vm/runtime/vm_version.cpp
! src/share/vm/utilities/copy.hpp
! src/share/vm/utilities/globalDefinitions.hpp
! src/share/vm/utilities/taskqueue.hpp

Changeset: 9cd8a2c2d584
Author:bobv
Date:  2011-02-02 11:54 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/9cd8a2c2d584

Merge

! src/os/linux/vm/os_linux.cpp

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[bob . vandette]

Changeset: 2f9d59b0fa5c
Author:bobv
Date:  2011-01-07 12:44 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/2f9d59b0fa5c

7009268: guarantee(middle - slop  start) failed: need enough space to divide up
Summary: Codebuffer can overflow on test with large number of calls
Reviewed-by: dholmes, collins

! src/share/vm/c1/c1_Compilation.cpp
! src/share/vm/c1/c1_Compilation.hpp

Changeset: 4537d449ba57
Author:bobv
Date:  2011-01-07 15:57 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/4537d449ba57

Merge

hg: jdk7/hotspot-rt/hotspot: 7007769: VM crashes with SIGBUS writing PerfData if tmp space is full

[bob . vandette]

Changeset: 02895c6a2f82
Author:bobv
Date:  2010-12-20 14:30 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/02895c6a2f82

7007769: VM crashes with SIGBUS writing PerfData if tmp space is full
Summary: Fill perfdata file with zeros to verify available disk space
Reviewed-by: coleenp, kamg

! src/os/linux/vm/perfMemory_linux.cpp

hg: jdk7/hotspot-rt/hotspot: 7004217: Remove IA64 workaround re-introduced with CR6953477

[bob . vandette]

Changeset: 6a2d73358ff7
Author:bobv
Date:  2010-12-02 14:00 -0500
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/6a2d73358ff7

7004217: Remove IA64 workaround re-introduced with CR6953477
Summary: gcc bug worksaround for IA64 no longer needed
Reviewed-by: andrew

! src/share/vm/interpreter/bytecodeInterpreter.cpp

hg: jdk7/hotspot-rt/hotspot: 2 new changesets

[bob . vandette]

Changeset: 3dc12ef8735e
Author:bobv
Date:  2010-10-07 15:12 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/3dc12ef8735e

6989297: Integrate additional portability improvements
Reviewed-by: vladidan, dholmes

! src/cpu/sparc/vm/globals_sparc.hpp
! src/cpu/x86/vm/globals_x86.hpp
! src/cpu/x86/vm/methodHandles_x86.cpp
! src/cpu/zero/vm/globals_zero.hpp
! src/os/linux/vm/attachListener_linux.cpp
! src/share/vm/includeDB_core
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/sharedRuntime.cpp
! src/share/vm/runtime/sharedRuntime.hpp

Changeset: 7491c8b96111
Author:bobv
Date:  2010-10-07 15:14 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/7491c8b96111

Merge

hg: jdk7/hotspot-rt/hotspot: 6953477: Increase portability and flexibility of building Hotspot

[bob . vandette]

Changeset: 126ea7725993
Author:bobv
Date:  2010-08-03 08:13 -0400
URL:   http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/126ea7725993

6953477: Increase portability and flexibility of building Hotspot
Summary: A collection of portability improvements including shared code support 
for PPC, ARM platforms, software floating point, cross compilation support and 
improvements in error crash detail.
Reviewed-by: phh, never, coleenp, dholmes

! agent/src/os/linux/ps_proc.c
! make/Makefile
! make/defs.make
! make/linux/makefiles/build_vm_def.sh
! make/linux/makefiles/buildtree.make
! make/linux/makefiles/defs.make
! make/linux/makefiles/gcc.make
! make/linux/makefiles/product.make
! make/linux/makefiles/sa.make
! make/linux/makefiles/saproc.make
! make/linux/makefiles/vm.make
! make/solaris/makefiles/defs.make
! src/cpu/sparc/vm/bytecodeInterpreter_sparc.inline.hpp
! src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp
! src/cpu/sparc/vm/c1_Runtime1_sparc.cpp
! src/cpu/sparc/vm/interpreterRT_sparc.cpp
! src/cpu/sparc/vm/javaFrameAnchor_sparc.hpp
! src/cpu/sparc/vm/templateTable_sparc.cpp
! src/cpu/x86/vm/bytecodeInterpreter_x86.inline.hpp
! src/cpu/x86/vm/c1_LIRGenerator_x86.cpp
! src/cpu/x86/vm/c1_Runtime1_x86.cpp
! src/cpu/x86/vm/frame_x86.cpp
! src/cpu/x86/vm/interpreterRT_x86_32.cpp
! src/cpu/x86/vm/javaFrameAnchor_x86.hpp
! src/cpu/x86/vm/templateTable_x86_32.cpp
! src/cpu/x86/vm/templateTable_x86_64.cpp
! src/os/linux/launcher/java_md.c
! src/os/linux/vm/os_linux.cpp
! src/os/solaris/vm/os_solaris.cpp
! src/os/windows/vm/os_windows.cpp
! src/os_cpu/linux_sparc/vm/thread_linux_sparc.cpp
! src/os_cpu/linux_x86/vm/os_linux_x86.cpp
! src/os_cpu/linux_x86/vm/thread_linux_x86.cpp
! src/os_cpu/linux_zero/vm/thread_linux_zero.cpp
! src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
! src/os_cpu/solaris_sparc/vm/thread_solaris_sparc.cpp
! src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
! src/os_cpu/solaris_x86/vm/thread_solaris_x86.cpp
! src/os_cpu/windows_x86/vm/os_windows_x86.cpp
! src/os_cpu/windows_x86/vm/thread_windows_x86.cpp
! src/share/vm/asm/codeBuffer.hpp
! src/share/vm/c1/c1_CodeStubs.hpp
! src/share/vm/c1/c1_Compilation.hpp
! src/share/vm/c1/c1_FrameMap.cpp
! src/share/vm/c1/c1_FrameMap.hpp
! src/share/vm/c1/c1_LIR.cpp
! src/share/vm/c1/c1_LIR.hpp
! src/share/vm/c1/c1_LIRGenerator.cpp
! src/share/vm/c1/c1_LIRGenerator.hpp
! src/share/vm/c1/c1_LinearScan.cpp
! src/share/vm/c1/c1_Runtime1.cpp
! src/share/vm/c1/c1_Runtime1.hpp
! src/share/vm/code/codeBlob.cpp
! src/share/vm/code/codeBlob.hpp
! src/share/vm/code/nmethod.hpp
! src/share/vm/code/vtableStubs.cpp
! src/share/vm/code/vtableStubs.hpp
! src/share/vm/compiler/disassembler.cpp
! src/share/vm/includeDB_compiler1
! src/share/vm/includeDB_core
! src/share/vm/interpreter/bytecodeInterpreter.cpp
! src/share/vm/interpreter/bytecodeInterpreter.hpp
! src/share/vm/interpreter/bytecodeInterpreter.inline.hpp
! src/share/vm/interpreter/interpreter.cpp
! src/share/vm/interpreter/interpreter.hpp
! src/share/vm/interpreter/oopMapCache.cpp
! src/share/vm/memory/allocation.cpp
! src/share/vm/memory/allocation.hpp
! src/share/vm/memory/genCollectedHeap.cpp
! src/share/vm/memory/generation.hpp
! src/share/vm/oops/arrayKlass.cpp
! src/share/vm/oops/arrayKlass.hpp
! src/share/vm/oops/arrayKlassKlass.cpp
! src/share/vm/oops/arrayKlassKlass.hpp
! src/share/vm/oops/compiledICHolderKlass.cpp
! src/share/vm/oops/compiledICHolderKlass.hpp
! src/share/vm/oops/constMethodKlass.cpp
! src/share/vm/oops/constMethodKlass.hpp
! src/share/vm/oops/constantPoolKlass.cpp
! src/share/vm/oops/constantPoolKlass.hpp
! src/share/vm/oops/cpCacheKlass.cpp
! src/share/vm/oops/cpCacheKlass.hpp
! src/share/vm/oops/generateOopMap.cpp
! src/share/vm/oops/klass.cpp
! src/share/vm/oops/klass.hpp
! src/share/vm/oops/klassKlass.cpp
! src/share/vm/oops/klassKlass.hpp
! src/share/vm/oops/oop.cpp
! src/share/vm/prims/jni.cpp
! src/share/vm/prims/jvmtiEnvThreadState.hpp
! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/frame.cpp
! src/share/vm/runtime/frame.hpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/java.cpp
! src/share/vm/runtime/javaFrameAnchor.hpp
! src/share/vm/runtime/os.cpp
! src/share/vm/runtime/os.hpp
! src/share/vm/runtime/sharedRuntime.cpp
! src/share/vm/runtime/sharedRuntime.hpp
! src/share/vm/runtime/sharedRuntimeTrans.cpp
! src/share/vm/runtime/signature.hpp
! src/share/vm/runtime/stubCodeGenerator.cpp
! src/share/vm/runtime/stubCodeGenerator.hpp
! src/share/vm/runtime/thread.cpp
! src/share/vm/runtime/thread.hpp
! src/share/vm/runtime/vm_version.cpp
! src/share/vm/runtime/vm_version.hpp
! src/share/vm/utilities/debug.cpp
! src/share/vm/utilities/globalDefinitions_gcc.hpp
! src/share/vm/utilities/macros.hpp
! src/share/vm/utilities/vmError.cpp
! src/share/vm/utilities/vmError.hpp