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