[9] RFR (M): 8139595: MethodHandles::remove_dependent_nmethod is not MT safe

[Aleksey Shipilev]

On 11/05/2015 04:54 PM, Vladimir Ivanov wrote:
> Regarding internal purging, add_dependent_methods doesn't iterate over
> the whole list, but looks for the relevant entry. It means that "purge"
> flag should force full iteration. I think that keeping them separately
> (add_dependent_methods() vs purge()) looks cleaner.

I think it is better to encapsulate the "piggybacking" behavior within
the DependencyContext itself, because that seems to be a
general/expected feature of DependencyContext class. It is weird to ask
class users to spell it out specifically.


> I could add "purge" flag and move the following code into
> add_dependent_methods:
> void DependencyContext::add_dependent_nmethod(nmethod* nm, bool do_purge
> = false) {
>    ...
>    if (has_unloaded_dependent() && do_purge) {
>      purge();
>    }
> }
> 
> Is it what you are suggesting?

Yes.


>>   * DependencyContext now has three methods: purge(), clear(), wipe(). I
>> have trouble understanding which method does what!

> There are basically 2 operations needed:
>   * purge(): only remove stale entries; no additional work is performed;

Oh. Should probably mention "stale" in the name. See e.g. Java-ish:
 WeakHashMap.expungeStaleEntries()
 ThreadLocal.expungeStaleEntries()
 WeakCache.expungeStaleEntries()


> wipe() is only for unit-testing purposes (see TestNmethodBucket): it
> just deallocates all nmethodBucket entries without touching nmethods. It
> is necessary since nmethod pointers are faked by the test.

Okay. Is there a convention how you should name the test-related methods
like these? I would expect something like debug_deallocate_buckets().

Thanks,
-Aleksey