openj9
openj9 copied to clipboard
eclipse-openj9
Build a graph of class loaders based on lifetime relationships
Each J9ClassLoader now maintains a set of class loaders known to live at least as long at it (outlivingLoaders). When a class is looked up in a loader L1, and the class that's found was defined by a different loader L2, L1 adds the class to its hash table, guaranteeing that L2 will outlive L1. Now when this occurs, L1 will additionally take note that L2 outlives it. In this way we construct a graph of loaders describing the lifetime relationships that have been observed so far. This graph can be searched to determine the full set of loaders that are known to outlive any particular loader. It would be possible to skip reifying this graph and to get the same result instead by scanning through the (potentially much larger) class hash tables.
Additionally, identify the permanent loaders by getting them from systemClassLoader, extensionClassLoader, and applicationClassLoader, but only once they're fully initialized and frozen. It's possible for the latter two to be replaced during bootstrapping.
Permanent loaders don't need an outliving loader set, since they're outlived only by other permanent loaders, so instead outlivingLoaders is set to a special value to indicate that the loader is permanent.
Permanence is propagated in that any loader that is found to outlive a permanent loader must also be permanent. This will be taken into account regardless of which is known first: the permanence or the lifetime relationship.
Permanent loaders are reported to the JIT so that it can take advantage of them when reasoning about class loader lifetimes. It is now possible to get a vector of pointers to all permanent loaders known so far by calling J9::Compilation::permanentLoaders().
At the moment, outlivingLoaders and permanentLoaders() are essentially unused, but soon they will both be used for RetainedMethodSet.
As of the time of opening, this PR is split into two commits.
The first commit is just the VM part of #21216, verbatim. I intend to remove those VM changes from that PR in favour of this one.
The second commit has further changes to identify and propagate permanence of class loaders. There are compiler changes here, but only enough to provide the permanentLoaders() method mentioned above, and most of those changes are just to make it work in JITServer.
The commits are separated only to make it clear what the additional changes are in the VM compared to what was already in #21216, for anybody who has already read the VM part of that PR. I will squash the commits before merge.
@gacholio, could you please review the VM changes? @mpirvu, could you please review the JIT changes?
Please see the note about commits at the end of the initial description
Alright @mpirvu, I understand the layering concern, and I didn't know that we could repeatedly disconnect and reconnect to the same server. Your alternate design sounds good to me, so I'll change up the JITServer part to do as you described. As for synchronization, might it make sense to use the sequencing monitor? There's already a critical section right there. That's what I was going to do before I decided to try to avoid needing to synchronize.
As for synchronization, might it make sense to use the sequencing monitor?
Yes, that's fine. Just add a comment for _numPermanentLoadersSent that it is protected by the sequencing monitor.
Squashed and addressed most of the inline comments from @mpirvu. I just wanted to get those out of the way before I make the larger change, though some may not still apply afterward
When a class is looked up in a loader L1, and the class that's found was defined by a different loader L2, L1 adds the class to its hash table, guaranteeing that L2 will outlive L1.
I don't believe that provides any such guarantee. That class from L2 may be unloaded, and when that occurs it should be removed from all class loaders.
That class from L2 may be unloaded
How? (without at the same time also unloading L2, L1, and all of the classes they've defined)
It's reachable from L1.
What about HCR? The graphs will need to be updated with new RAM class addresses.
For extended HCR (which I hope is still deprecated), the class hash tables will be updated to replace the original class with the new one. But there are no class pointers in outlivingLoaders, so the replacement doesn't need to happen there, and the replacement class really should have the same defining loader as the original, so I don't think that any adjustment should be needed at all
Section 12.7 of JLS (version Java 21) says:
A class or interface may be unloaded if and only if its defining class loader may be reclaimed by the garbage collector
So the class defined by L2 cannot be unloaded unless L2 is also garbage. Putting that class in the class table of L1 should not (by itself) prevent unloading of L2 and thus provides no guarantee that L2 outlives L1.
It does prevent unloading of L2 (for at least as long as L1 is live) because it ensures that L2 will be reachable. There's a path from L1 to the class defined by L2, and then to L2 itself.
That JLS quote is just another way of stating that a class and its defining loader have the exact same lifetime, which I have already said elsewhere in this PR, and which, if anything, supports my position. In particular, it doesn't mean that classes can be unloaded despite still being reachable. That would be a disaster.
OK, I can demonstrate that we do have such a guarantee, and it's even a logical consequence of the spec.
There's a requirement in JVMS sections 5.3.1 and 5.3.2 that once a class with a given name has been successfully found via an initiating loader, later attempts to look up the same name in the same initiating loader will find the the same class:
First, the Java Virtual Machine determines whether [the initiating loader] has already been recorded as an initiating loader of a class or interface denoted by N. If so, this class or interface is C, and no class creation is necessary.
This implies that a class must outlive all initiating loaders from which it has previously been found, since as long as such an initiating loader is live, the program could ask it again for a class of the same name. Then because the class and its defining loader have the same lifetime, the defining loader also outlives all of those initiating loaders.
Pushed an update addressing comments from Keith. Most of the changes are just white space.
It's just a trivial conflict in the JITServer stream version number, so it doesn't interfere with review. I'm holding off in an attempt to reduce the number of times I need to resolve this conflict. I still need to finish changing the approach to JITServer before this will be ready for PR testing anyway
finish changing the approach to JITServer before this will be ready
In draft mode until then.
@mpirvu, I've updated the JITServer approach.
I avoided rebasing, so the diff hasn't pulled in unrelated changes from other PRs. I'll rebase now.
Updated cleanUpClassLoader() and freeClassLoader() based on Keith's feedback, but there is apparently now a conflict, so I'll resolve that
Please correct the typo in the (third) commit message: "least as long at it" -> "least as long as it".
Moved the logic that frees outlivingLoaders in freeClassLoader() as requested, and corrected the typo in the commit message.
@gacholio Please have another look to confirm you're still happy with the changes here.