RFR: 8317534: GenShen: Remove unused code [v2]

William Kemper wkemper at openjdk.org
Tue Oct 10 00:12:29 UTC 2023


On Thu, 5 Oct 2023 18:27:23 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Restore deleted comment on correct method
>
> src/hotspot/share/gc/shenandoah/shenandoahAgeCensus.hpp line 113:
> 
>> 111:   // This uses the data in the ShenandoahAgeCensus object's _global_age_table and the
>> 112:   // current _epoch to compute a new tenuring threshold, which will be remembered
>> 113:   // until the next invocation of compute_tenuring_threshold.
> 
> Could you please move this documentation to `compute_tenuring_threshold()` further below. Thanks!

Done.

> src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp line 139:
> 
>> 137: 
>> 138:   void set_soft_max_capacity(size_t soft_max_capacity) {
>> 139:     _soft_max_capacity = soft_max_capacity;
> 
> Not only the accessor, but it seems like the `_soft_max_capacity` field in `ShenandoahGeneration` is vestigial and can be deleted?
> 
> A cursory look seemed to indicate that a field with a similar name (similar accessor, but confusingly different field name) is used for the purpose for that purpose for the whole heap, and the generation's field is obsolete/vestigial. I may have missed some uses in my inspection, but you can check if deleting the field is ok (it's only used in logging where it seems not to have much value?).

Hmm, this field is used by the heuristics. This looks like a regression to me. The soft max heap size can be given as a command line argument, but it is also supposed to be a 'managed' option that can be changed with jmx. I will open a separate ticket for this.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 859:
> 
>> 857:   void clear_cards_for(ShenandoahHeapRegion* region);
>> 858:   void dirty_cards(HeapWord* start, HeapWord* end);
>> 859:   void clear_cards(HeapWord* start, HeapWord* end);
> 
> These aren't used, but in looking through the code for marking and clearing cards in `ShenandoahDirectCardMarkRememberedSet::mark_range_as_...` I realize we might want to see if we want to use vector instructions or use `memset`, when the range is sufficiently large (future work).

That's a good idea!

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/335#discussion_r1351010105
PR Review Comment: https://git.openjdk.org/shenandoah/pull/335#discussion_r1351009661
PR Review Comment: https://git.openjdk.org/shenandoah/pull/335#discussion_r1351011714


More information about the shenandoah-dev mailing list