RFR: 8234654: ZGC: Only disarm NMethods when marking/relocating code roots

Stefan Karlsson stefan.karlsson at oracle.com
Mon Dec 9 21:29:39 UTC 2019


Reworked the patch a bit, because I wanted to move _disarm_nmethod out 
from the iterator and into the closures:

https://cr.openjdk.java.net/~stefank/8234654/webrev.01.delta/
https://cr.openjdk.java.net/~stefank/8234654/webrev.01/

- Ripped out _disarm_nmethod from the ZRootsIterator.

- Added a should_disarm_nmethod() virtual function to the closure. The 
mark and relocation closures return true. Had a version that introduced 
a ZRootsIterator::do_code_blob analogous to ZRootsIterator::do_thread, 
but we don't need that flexibility right now, so backed away from that 
in favor of more straight forward code.

- Removed closure indirection in ZRootsIteratorCodeBlobClosure.

- Changed to use ZNMethod::nmethod_oops_do instead of nm->oops_do and 
nm->fix_oop_relocations, since it's faster .

- Changed to use ZNMethod::disarm(nm) instead of _bs->disarm(nm), since 
it hides the _bs retrieval.

- Added armed state assert.

This passes tier1-7 of ZGC testing.

Sat down with Per for the final version of this. Either we fold this 
patch into his change, or I'll add this as a patch afterwards.

Thanks,
StefanK


On 2019-12-09 10:17, Per Liden wrote:
> Here's an updated webrev, which removes the "assert(is_armed, ...)" 
> BarrierSetNMethod::disarm(), as it's a bit too strict. Also, added the 
> "if (ZNMethod::is_armed(nm))" to the ZNMethodUnlinkClosure, so avoid 
> walking to oops if it's already disarmed.
>
> http://cr.openjdk.java.net/~pliden/8234654/webrev.1
>
> /Per
>
> On 11/22/19 3:38 PM, Per Liden wrote:
>> Thanks Erik!
>>
>> /Per
>>
>> On 11/22/19 3:35 PM, erik.osterlund at oracle.com wrote:
>>> Hi Per,
>>>
>>> Looks good.
>>>
>>> Thanks,
>>> /Erik
>>>
>>> On 11/22/19 3:03 PM, Per Liden wrote:
>>>> ZRootIterator will currently always try to disarm on-stack 
>>>> NMethods. Strictly speaking, we should only do this when 
>>>> marking/relocating code roots, not when e.g. iterating the heap.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234654
>>>> Webrev: http://cr.openjdk.java.net/~pliden/8234654/webrev.0
>>>>
>>>> /Per
>>>




More information about the hotspot-gc-dev mailing list