RFR: 8324649: Shenandoah: replace implementation of free set [v59]
Kelvin Nilsen
kdnilsen at openjdk.org
Wed May 15 14:27:46 UTC 2024
On Tue, 14 May 2024 01:08:01 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fix copyright notice on test file
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 93:
>
>> 91: void dump_bitmap_row(ssize_t idx) const;
>> 92: void dump_bitmap_range(ssize_t start_idx, ssize_t end_idx) const;
>> 93: void dump_bitmap_all() const;
>
> In the first case you can may be say `region_idx` instead of `idx` ?
>
> Also, it seems `dump_bitmap()` would be sufficient (at least for the case of `dump_bitmap_all()`, although in general for all).
>
> However, given that these are private methods that will only be found in the implementation and nowhere else in its clients' code, may be current naming is ok (indeed, it looks like the first two are work functions in service of the last).
>
> PS: I don't see any current usage of `dump_bitmap_all()`. May be you wanted it in some debug tracing code or something, or keeping it for possible future debugging usage? (Or may be my naive web-page search of this review page is naive/faulty.) It's perhaps useful to keep around for debug/tracing though.
Thanks. Making these changes to argument names, and conditionalizing this code on #ifndef PRODUCT
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1601749296
More information about the hotspot-gc-dev
mailing list