RFR: 8234654: ZGC: Only disarm NMethods when marking/relocating code roots
per.liden at oracle.com
Tue Dec 10 08:07:18 UTC 2019
Thanks for reviewing. I'll fold the suggested changes into the patch.
On 12/9/19 10:29 PM, Stefan Karlsson wrote:
> Reworked the patch a bit, because I wanted to move _disarm_nmethod out
> from the iterator and into the closures:
> - 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.
> 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.
>> On 11/22/19 3:38 PM, Per Liden wrote:
>>> Thanks Erik!
>>> On 11/22/19 3:35 PM, erik.osterlund at oracle.com wrote:
>>>> Hi Per,
>>>> Looks good.
>>>> 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
More information about the hotspot-gc-dev