RFR: 8334147: Shenandoah: Avoid taking lock for free set logging [v6]

Xiaolong Peng xpeng at openjdk.org
Thu Jun 27 15:48:14 UTC 2024


On Thu, 27 Jun 2024 08:22:54 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Make log_status_with_heap_locked public
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 344:
> 
>> 342:   void recycle_trash();
>> 343:   void log_status();
>> 344:   void log_status_with_heap_locked();
> 
> Suggestion: this can be just `log_status_under_lock`. We already have other methods with `*_under_lock`.

Checked some code in this name pattern:

HeapWord* ShenandoahHeap::allocate_memory_under_lock(ShenandoahAllocRequest& req, bool& in_new_region) {
  // If we are dealing with mutator allocation, then we may need to block for safepoint.
  // We cannot block for safepoint for GC allocations, because there is a high chance
  // we are already running at safepoint or from stack watermark machinery, and we cannot
  // block again.
  ShenandoahHeapLocker locker(lock(), req.is_mutator_alloc());
  return _free_set->allocate(req, in_new_region);
}


It assumes lock is not acquired, lock is acquired in the method, I can change it to this pattern to align the naming conversions/patterns, but won't be a simple renaming.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19915#discussion_r1657366587


More information about the hotspot-gc-dev mailing list