RFR: 8310537: Fix -Wconversion warnings in gcUtil.hpp

Axel Boldt-Christmas aboldtch at openjdk.org
Thu Jun 22 12:32:04 UTC 2023


On Thu, 22 Jun 2023 12:08:08 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/gcUtil.cpp line 122:
>> 
>>> 120:   _sum_xy = _sum_xy + x * y;
>>> 121:   _mean_x.sample((float)x);
>>> 122:   _mean_y.sample((float)y);
>> 
>> If this narrowed more than some small epsilon the behaviour would be strange. Seems like the actual interface here should be to sample float precision `x` and `y`.  (`void LinearLeastSquareFit::update(float x, float y)`)
>> 
>> I have no objection for doing it this way, just an observation of `LinearLeastSquareFit`
>> 
>> I know from my own experiments with `-Wconversions` that we have many areas where it is the interface that seems wrong. (Sometimes moving from int -> long -> int -> long, just to fit different subcomponent interfaces)
>
> Oh you've had experiences with -Wconversions? Thank you, I can ask you questions. I've tried to fix the interface when possible to be more precise with int types. In this case, it was float so not as sure if this is the right thing to do.  I'm not sure I understand your comment.  LinearLeastSquare fit seems determined to take double, and AdaptiveWeightedAverage uses float. I'm not sure how to change this to make it safer.  My usual strategy would be to make the AdaptiveWeightedAverage take double but then the fall out from that would be more than I can deal with.  What do you think?

What I mean is that we are doing a least square fit on a bunch of datapoints. The algorithm requires the mean, sum and sum of squares. If the datapoints truly required double precision or magnitude then storing the sum and sum of square in a double will overflow and/or be inaccurate. 

AdaptiveWeightedAverage having a float interface strengthens the case that the samples should be floats, as the running mean should never overflow or underflow the samples `min sample ≤ mean sample ≤ max sample`, (may lose precision, but if the samples are float precision, it seems resonable that the mean also have this precision)

It was not about safety but about having the interface express the capabilities of the class. It can sample float x,y points and provide a least square fit linear function`y(x)`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14585#discussion_r1238454835


More information about the hotspot-gc-dev mailing list