RFR: 8339648: ZGC: Division by zero in rule_major_allocation_rate [v2]
Axel Boldt-Christmas
aboldtch at openjdk.org
Mon Sep 16 14:17:07 UTC 2024
On Mon, 16 Sep 2024 08:15:38 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:
>> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Adjust division following suggestion by xmas
>
> src/hotspot/share/gc/z/zDirector.cpp line 491:
>
>> 489: // Calculate the GC cost for each reclaimed byte
>> 490: const double current_young_gc_time_per_bytes_freed = double(young_gc_time) / double(reclaimed_per_young_gc);
>> 491: const double current_old_gc_time_per_bytes_freed = reclaimed_per_old_gc == 0 ? std::numeric_limits<double>::infinity() : double(old_gc_time) / double(reclaimed_per_old_gc);
>
> How about using some parentheses? To my understanding, the division has a higher precedence than the ternary conditional expression. See: https://en.cppreference.com/w/cpp/language/operator_precedence
I do not mind parentheses. But ternary are the lowest precedence (if you do not count the `,` which I would almost always say is wrong to use without a surrounding `() / [] / {}`), so to me it seems superfluous.
Just to clarify the intent of this code is what we are getting with a higher precedence on division. That is:
const double current_old_gc_time_per_bytes_freed = ((reclaimed_per_old_gc == 0) ? (std::numeric_limits<double>::infinity()) : (double(old_gc_time) / double(reclaimed_per_old_gc)));
_Side Note:_
I also think I prefer immediately invoked lambdas when the ternaries get this long.
const double current_old_gc_time_per_bytes_freed = [&]() {
if (reclaimed_per_old_gc == 0) {
return std::numeric_limits<double>::infinity();
}
return double(old_gc_time) / double(reclaimed_per_old_gc);
}();
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20888#discussion_r1761248058
More information about the hotspot-gc-dev
mailing list