RFR: 8337269: G1ConfidencePercent interpreted inconsistently [v2]

[Kim Barrett]

On Fri, 11 Oct 2024 13:18:45 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that removes the inconsistency in interpretation of G1ConfidencePercent; higher confidence means that G1 should use less safety margin (i.e. part of the variance).
>> 
>> Other than that it does not change the uses for it.
>> 
>> Testing: gha
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - * whitespace fix
>  - * kbarrett review

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/g1/g1Predictions.hpp line 53:

> 51: public:
> 52:   G1Predictions(double sigma_scale) : _stddev_scale(sigma_scale) {
> 53:     assert(sigma_scale >= 0.0, "must be larger than or equal to zero");

The argument uses "sigma" prefix, but everywhere else it's "stddev".

src/hotspot/share/gc/g1/g1Predictions.hpp line 53:

> 51: public:
> 52:   G1Predictions(double sigma_scale) : _stddev_scale(sigma_scale) {
> 53:     assert(sigma_scale >= 0.0, "must be larger than or equal to zero");

The message text doesn't provide any information beyond what what's given by the stringized predicate
code.  It even makes things a bit worse, since I had to verify the two were consistent.  I think just "precondition"
or "must be" or one of those common messages would actually be preferable here.

-------------

PR Review: https://git.openjdk.org/jdk/pull/21448#pullrequestreview-2367510228
PR Review Comment: https://git.openjdk.org/jdk/pull/21448#discussion_r1800014732
PR Review Comment: https://git.openjdk.org/jdk/pull/21448#discussion_r1800016820