Summary

The heap sampling spec contained three items that should be updated due to being either using the wrong term, being too specific, and adding information about expected results.

The following CSR is to:

1) replace the term "rate" with "interval" as it makes more sense

2) provide a detailed sentence about what to expect when setting the interval to 0

3) remove the implementation details about how the sampling interval is handled by a given implementation

Goals

Reword the description of the SetHeapSamplingRate to allow:

• A redefinition of rate to interval, across variables and method name
• Use any implementation that provides an average sampling rate after a large number of samples (thus not automatically a geometric means)
• Provide more information about what to expect with an interval of 0, ie that the system will eventually start generating an event for each allocation

Motivation

The current description is a bit too restrictive/incomplete for the underlying implementation, which might provoke a poor performance effect. Providing a bit more leeway enables implementations to determine what works best without losing the real goal of the heap sampling system: providing a means to monitor heap allocations with a low overhead.

Description

Change the description of the SetHeapSamplingRate, renaming rate to interval

Instead of explicitly saying geometric rate, the description now is: " By default, the sampling interval is set to 512KB. The sampling is semi-random to avoid pattern-based bias and provides an approximate overall average interval over long periods of sampling.."

Sampling rate is replaced with sampling interval

Since a rate is more a frequency of obtained samples out of a given metric size, sampling interval makes more sense. Rewording the spec and implementation to sampling interval should be done.

Provide information about what happens when the interval is set to 0

When updating the sampling interval, the implementation actually only starts converging to that average once the TLAB gets replaced. This is a problem when the description for setting the interval to 0 is "setting the interval to 0 samples each allocation" since this is true ONCE the TLAB gets replaced.

To fix this, the spec fix is to make explicit that setting the interval will not be taken into account straight away.

The rephrasing is: Setting to 0 will cause an event to be generated by each allocation supported by the system once the new interval is taken into account.

Note that updating the new sampling interval might take various number of allocations to provoke internal data structure updates. Therefore it is important to consider the sampling interval as an average. This includes the interval 0, where events might not be generated straight away for each allocation.

Implementation details

The only change to the implementation is the change of "rate" to "interval", which is a trivial and very low risk change.

Alternatives

Leave the spec as is and force the implementation of the system to be constrained to a geometric variable mean. Also leave the name of "rate" instead of interval in place, perhaps confusing certain users. Finally, leave the ambiguous case for a zero interval, where the user has to wait for it to taken into account.

Testing

Nothing really changes for the in place implementation so testing does not need to change.

Risks and Assumptions

Nothing really, this is generalizing the spec a bit for other implementations and changing the name of a variable in a JVMTI method for a newly added JDK11 method.

More information

The new spec can be found in the jvmti.diff.html attached to this issue or the current webrev for this is here: http://cr.openjdk.java.net/~jcbeyler/8205725/webrev.03 /

The spec diff is here: http://cr.openjdk.java.net/~jcbeyler/8205725/webrev.03/src/hotspot/share/prims/jvmti.xml.udiff.html