RFR: 8210498: nmethod entry barriers

[Erik Österlund]

Hi Per and Roman,

Thank you for having a look at this.
I went with Roman's suggestion and made this an explicit opt-out 
instead. But I let ModRef pass in NULL, as none of the barrier sets in 
that category currently use nmethod entry barriers. When they start to 
do so, that can be easily changed.

Incremental:
http://cr.openjdk.java.net/~eosterlund/8210498/webrev.00_01/

Full:
http://cr.openjdk.java.net/~eosterlund/8210498/webrev.01/

Thanks,
/Erik

On 2018-10-08 08:48, Roman Kennke wrote:
> I agree. I wouldn't mind to keep it simple and pass NULL though. I'm not
> a big fan of default arguments anyway.
>
> Roman
>
>> On 2018-10-05 17:07, Erik Österlund wrote:
>>> Hi,
>>>
>>> In order to implement concurrent class unloading for ZGC, a mechanism
>>> will be required to arm nmethods in a safepoint such that the first
>>> entry into an nmethod after the pause triggers a barrier before
>>> continuing.
>>>
>>> The barrier will be used for:
>>>     * Patching immediate oops, and keeping phantomly reachable oops
>>> alive through the nmethod entry
>>>     * Catching calls from stale IC caches (due to class unloading) into
>>> nmethods that are being unloaded (containing broken oops and
>>> metadata), and lazily clean the IC cache and re-resolve the call.
>>>
>>> This implementation is for x86_64 and is based on the prototyping work
>>> of Rickard Bäckman. So big thanks to Rickard, who will co-author of
>>> this patch.
>>> The way it works is that there is a cmp #offset(r15_thread),
>>> simm32_epoch); je #continuation; inserted into the verified entry of
>>> nmethods that conditionally calls a stub trampolining into the VM.
>>> When the simm32_epoch is not equal to some TLS-local field, then a
>>> slow path is taken to run some barrier inside of the VM.
>>> By changing this TLS-local field in the safepoint, all verified
>>> entries will take the slow path into the VM when run, where the
>>> barrier is eventually disarmed by patching the simm32_epoch value of
>>> the instruction stream to the current globally correct value.
>>>
>>> In terms of abstractions, to utilize this mechanism, you need to
>>> create your BarierSet with an BarrierSetNMethod helper object, just
>>> like the BarrierSetAssembler, BarrierSetC1 and BarrierSetC2. The
>>> BarrierSetNMethod base class handles the barrier itself. It has a
>>> virtual function, nmethod_entry_barrier, which gets called by the slow
>>> paths. So each user of this mechanism inherits from BarrierSetNMethod
>>> and puts its barrier in there.
>>>
>>> The OSR nmethods do not have a verified entry. Therefore, the barrier
>>> is run explicitly in the runtime before calling into OSR nmethods to
>>> achieve the same effect of trapping new calls into nmethods.
>>>
>>> The nmethod entry barrier returns true/false depending on whether the
>>> called nmethod may be entered or not. This allows lazily cleaning
>>> nmethod IC caches during unloading, which invokes the same helper
>>> method that is patched into the verified entry when made not entrant.
>>>
>>> Eventually, I would like to also use this mechanism to remove the
>>> nmethod hotness counter maintenance we today perform in safepoint
>>> cleanup instead. These nmethod entry barriers can provide better
>>> heuristics for whether nmethods are used or not. But that would
>>> require buy-in from maintainers of other platforms. So let's take one
>>> step at a time.
>>>
>>> For the curious reader thinking that this might be an expensive
>>> operation, it is not. This check in the nmethod verified entry is very
>>> predictable and cheap. And because of use of inlining, the checks
>>> become more infrequent. In my evaluation, it has gone completely under
>>> the radar in benchmarking (running this with my current version of
>>> concurrent class unloading in ZGC).
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8210498/webrev.00/
>> I reviewed this before Erik sent it out, so my comments have already
>> been addressed. Just found one additional thing, the BarrierSet
>> constructor now looks like this:
>>
>> 96   BarrierSet(BarrierSetAssembler* barrier_set_assembler,
>> 97              BarrierSetC1* barrier_set_c1,
>> 98              BarrierSetC2* barrier_set_c2,
>> 99              const FakeRtti& fake_rtti,
>> 100             BarrierSetNMethod* barrier_set_nmethod = NULL) :
>> 101     _fake_rtti(fake_rtti),
>> 102     _barrier_set_assembler(barrier_set_assembler),
>> 103     _barrier_set_c1(barrier_set_c1),
>> 104     _barrier_set_c2(barrier_set_c2),
>> 105     _barrier_set_nmethod(barrier_set_nmethod) {}
>>
>> I find it odd that the fake_rtti argument comes in the middle of the
>> barrier_set_* arguments. I can see that you don't want GC barrier sets
>> to have to supply NULL if they don't have a nmethod barrier set, but
>> maybe we can have two constructors instead, like:
>>
>> BarrierSet(BarrierSetAssembler* barrier_set_assembler,
>>             BarrierSetC1* barrier_set_c1,
>>             BarrierSetC2* barrier_set_c2,
>>             const FakeRtti& fake_rtti) :
>>      _fake_rtti(fake_rtti),
>>      _barrier_set_assembler(barrier_set_assembler),
>>      _barrier_set_c1(barrier_set_c1),
>>      _barrier_set_c2(barrier_set_c2),
>>      _barrier_set_nmethod(NULL) {}
>>
>> BarrierSet(BarrierSetAssembler* barrier_set_assembler,
>>             BarrierSetC1* barrier_set_c1,
>>             BarrierSetC2* barrier_set_c2,
>>             BarrierSetNMethod* barrier_set_nmethod,
>>             const FakeRtti& fake_rtti,) :
>>      _fake_rtti(fake_rtti),
>>      _barrier_set_assembler(barrier_set_assembler),
>>      _barrier_set_c1(barrier_set_c1),
>>      _barrier_set_c2(barrier_set_c2),
>>      _barrier_set_nmethod(barrier_set_nmethod) {}
>>
>> What do you think?
>>
>> Other than that, this looks good to me.
>>
>> cheers,
>> Per
>>
>>> Bug ID:
>>> https://bugs.openjdk.java.net/browse/JDK-8210498
>>>
>>> Thanks,
>>> /Erik