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.
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 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? > 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). Thanks, Severin
