> On Jan 22, 2020, at 11:06 AM, Mandy Chung <mandy.ch...@oracle.com> 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