RFR: 8291718: Remove mark_for_deoptimization from klass unloading [v5]

[Dean Long]

On Thu, 4 Aug 2022 15:06:22 GMT, Axel Boldt-Christmas <duke at openjdk.org> wrote:

>>> You are correct that nm should all be at least `unloading`, as if a klass is unloading all dependent `nmethods` should be unloading. So this code should boil down to:
>>> 
>>> ```c++
>>> void DependencyContext::remove_all_dependents() {
>>>   nmethodBucket* b = dependencies_not_unloading();
>>>    set_dependencies(NULL);
>>>    assert(b == nullptr, "All dependents should be unloading");
>>> }
>>> ```
>>> 
>>> As `dependencies_not_unloading()` will unlink and release every dependent that is `unloading`. Have run stress tests on that change and no violations were found. Will push and test this change.
>> 
>> From testing this seems not to be the case. Will investigate some more. So there are dependents that do no become `unloading` when the klass unloads.
>
> I would like to find out more in detail what you want to ensure, or situation you are afraid that might occur. 
> 
> I've tried to investigate what types of dependent nmethods are not `unloading` when the the klass with a dependency is unloaded. I suspect it has to do with class loading and compilation order. From what I understand the nmethod will only be unloading if it has a reference to a dead oop inside it. So the compiler must generate code which is dependent without having an oop baked in. 
> 
> I do find it difficult to reason about the lifetime of these dependencies (at least the logical part, the release, purge cycle of the memory seems pretty straight forward) and nmethods (the sweeper removal does make the nmethod lifecycle a lot easier to reason about tough). 
> 
> I am really interested in what you think. As this change is just always running an edge case which can happen in mainline. This just makes it default. Hotspot seems to have relied on this interaction being this way for some time. 
> 
> The windows machines finally woke up and the patch passed tier 1-7

My concern was that we could be removing the dependencies from nmethods that are still using them.  But if that was the case, the old code would have the same problem.

-------------

PR: https://git.openjdk.org/jdk/pull/9713