RFR: 8291718: Remove mark_for_deoptimization from klass unloading

[Axel Boldt-Christmas]

On Tue, 2 Aug 2022 21:30:14 GMT, Dean Long <dlong at openjdk.org> wrote:

>> @stefank suggested creating a separate issue for this part of [JDK-8291237](https://bugs.openjdk.org/browse/JDK-8291237) 
>> 
>> Description from the original issue: 
>>> The klass unloading code currently sets the mark for deoptimzation flag on nmethods that depend on the klass being unloaded. That however is all it does, there is no guarantee that those nmethods will become not entrant and make_deoptimized. The current implementation may if some future deoptimization happens to trigger see these marks as it scans the whole code cache, but it is never triggered from klass unloading. 
>>> 
>>> Me and [~eosterlund] discussed this and this behaviour seems like some leftover from earlier versions of klass unloading. There should be no reason to eliminate the nmethods that depend on unloaded klasses except to eliminate dead code. It is probably better to let other mechanisms decide if an nmethod with dead code should be deoptimized than the klass unloading. 
>>> 
>>> It might be a future improvement to have some stats which record that an nmethod had some dependency to an unloaded klass which might effect the heuristic for selecting what methods are deoptimized.
>> 
>> Testing: Tier 1-3, 4-7 non-windows
>> Windows tier 4-7 still queued
>
> src/hotspot/share/code/dependencyContext.cpp line 221:
> 
>> 219:    int removed = 0;
>> 220:    while (b != NULL) {
>> 221:      b = release_and_get_next_not_unloading(b);
> 
> If we are being called by class unloading, finding an nmethod with nm->is_alive() returning true would be fatal, wouldn't it?  Could we make it clear this function is used for unloading, and add an assert that !nm->is_alive() && nm->has_flushed_dependencies()?

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.

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

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