On 4/3/18 10:09 PM, Bob Vandette wrote:
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 *<li> 1. All processes, including the current process within a container.
<ol> includes the numbering. You can drop "1." and other numbers.
42 *<li> or
This adds a bullet. Maybe dropping this line.
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?
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?
jdk/internal/platform/cgroupv1/Metrics.java
274 return SubSystem.getLongValue(cpuacct, "cpuacct.usage");
Should this be an instance method? like cpuacct.getLongValue("cpuacct.usage");
final field name can be made all caps.
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/
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.
% 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.
Mandy
