RFR (L): 8230706: Waiting on completion of strong nmethod processing causes long pause times with G1

[Thomas Schatzl]

Hi Stefan,

On 27.09.19 23:16, Stefan Johansson wrote:
> Hi Thomas,
> 
>> 25 sep. 2019 kl. 12:42 skrev Thomas Schatzl <thomas.schatzl at oracle.com>:
>>
>> Hi all,
>>
>>   can I have reviews for this change that fixes a regression introduced in jdk11?
[..]
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8230706
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8230706/webrev/
> 
> Thanks for hunting down and fixing this problem. The change looks good in general, but I have some comments:
> src/hotspot/share/code/nmethod.cpp
> —
> I would like the bit-operations to be hidden in functions, something like:
>    is_claimed(nmethod* nm) { return ((uintptr_t)nm & 0x1) != 0;}
>    claim(nmethod* nm) { return (nmethod*)((uintptr_t)nm | 0x1); }
>    mask_claimed(nmethod* nm) { return (nmethod*)((uintptr_t)nm & ~0x1); }
> The names might be improved, but you get my point.
> —
> 
> src/hotspot/share/gc/g1/g1CodeBlobClosure.hpp
> —
>    55   // Do we need to collect nmethods if they have already been claimed.
>    56   bool is_strong_iteration() const { return _pss != NULL; }
> 
> I’m not sure about this name, especially since it won’t be true for all ”strong” closures, just the ones during initial mark right? Naming is hard, so I’m not sure I have a better suggestion overall, but what do you think about calling this method should_collect_strong_nmethods()?
> ---
> 
> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> —
> 3845   class G1MarkStrongNMethodsTask : public AbstractGangTask {
> 3846     G1ParScanThreadStateSet* _per_thread_states;
> 3847     volatile uint _claim;
> 3848   public:
> 3849     G1MarkStrongNMethodsTask(G1ParScanThreadStateSet* per_thread_states)
> 3850     : AbstractGangTask("Remark strong nmethods"), _per_thread_states(per_thread_states), _claim(0) { }
> 3851
> 3852     void work(uint worker_id) {
> 3853       G1RemarkStrongNMethodsClosure cl(worker_id);
> 3854       _per_thread_states->iterate(&cl, &_claim);
> 3855     }
> 3856   } cl(per_thread_states);
> 
> Move the G1MarkStrongNMethodsTask definition outside the function, it think that is more along the lines what we usually do.
> —

All fixed in new webrev:

http://cr.openjdk.java.net/~tschatzl/8230706/webrev.0_to_1 (diff)
http://cr.openjdk.java.net/~tschatzl/8230706/webrev.1 (full)

Rerunning hs-tier1-5, almost done

Thanks,
   Thomas