RFR: 8218974: Free GC native structures in nmethod::flush
per.liden at oracle.com
Thu Feb 21 12:18:38 UTC 2019
On 2/18/19 9:11 AM, Erik Österlund wrote:
> Hi Per,
> On 2019-02-18 07:44, Per Liden wrote:
>> Hi Erik,
>> On 02/14/2019 12:55 PM, Erik Österlund wrote:
>>> An nmethod goes from being is_alive() to being !is_alive() and
>>> eventually being freed in nmethod::flush. Native structures for
>>> nmethods are freed in nmethod::flush when we free the nmethod. Except
>>> for a few things, including GC data. This enhancement proposes to fix
>>> that to make the life cycle of nmethods and their native data more
>>> In particular ZGC has per-nmethod data. The data is removed when
>>> unlinking nmethods, as opposed to when they are deleted. This is a
>>> bit awkward and makes things more difficult than they need to be.
>>> This patch adds a new CollectedHeap::flush_nmethod() function. In
>>> there ZGC deletes its attached GC data.
>> Do we need to introduce a new flush_nmethod()? Would it instead be
>> possible to move/adjust where unregister_nmethod() is called to get
>> the same effect? When just looking at the API, the relationship
>> between unregister and flush is not super obvious. Determining which
>> one will be called first and what a GC allowed/supposed to do in each
>> of them kind of requires you to inspect the call-sites.
> I think of it this way: unregister_nmethod is tied to the lifecycle of
> the nmethod oops, and flush_nmethod is for the nmethod itself.
> In particular, we call unregister_nmethod when an nmethod dies (becomes
> !is_alive()). When an nmethod has died, the oops should not be retained.
> In fact, when the nmethod becomes unloaded, it dies specifically because
> the oops are dead, forcing us to kill the nmethod. Then we unregister to
> tell the GC not to look at those oops again.
> If we moved unregister_nmethod to nmethod::flush, we would keep around
> nmethods with broken oops in GC data structures, and the GC could no
> longer trust those data structures, unless we rewrote them to take into
> consideration that the oops they maintain could be dead if the host
> nmethod has silently died. But I don't think that would be an improvement.
> Because of this, I think it is wise to separate between GC events for
> the nmethod dying, and being deleted, because they have different
I hear you. I like how this simplifies the nmethod data life cycle. Just
one minor thing, ZNMethodTable::lock_for_nmethod() could now be just:
Other than that, look good.
We might want to think about how/if this relates to the BarrierSet::on_*
functions, with regards to naming and where they live. But that's a
More information about the hotspot-gc-dev