HHH-18011 Parallel entity enhancement causes a classloader cache stampede #8263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR might look scary, but let me break it down. Best to look at the first commit in isolation.
CoreTypePool and ModelTypePool are two new types, however they are essentially an extension of the ByteBuddy core type pool with some additional caching capabilities.
There are two of them as the strategies we want to use are best kept different, so I created different types to best represent their purpose - and to benefit from typesafety in regards to which one needs to be used in each case.
The main difference is that CoreTypePool is meant to cache introspection of JDK classes and mapping annotations: these are classes which are allowed to be loaded, so we refer to their actual java Class which is slightly more efficient. For all other classes however we force to parse the bytecode - this is expensive but safer, to not risk loading a class before its enhancement.
(Before this patch each and every symbol was parsed from bytecode, using the default ByteBuddy TypePool and also not doing a great job of caching repeated lookups for the same symbols).
The remaining changes relate to actually using these two new TypePool(s) over the default ones, and allows also for an alternative construction of
EnhancerImplwhich passes these pools explicitly: the idea is that runtimes which need to repeadly run multiple enhancements can reuse theCoreTypePool's hot caches, since its cache content doesn't have references to the application being enhanced.The second commit simply moves some constants from
EnhancerImplinto a different holder class, again to allow reuse: those constants are not at all cheap to compute and were being re-initialized on each enhancer run; this might be fine for our build tools, but moving them out allows other runtimes or integrators to be a bit smarter about this.There were plenty of existing integration tests; including odd situations like HHH-13343 so we should be covered.
https://hibernate.atlassian.net/browse/HHH-18011