DependencyManager improvement #1539
Merged
Conversation
This file contains hidden or 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
Also clean up UT, as commented out lines in fact hide the issues; revert the assertions and do assert.
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 change affects the transitive dependency manager the most (in positive way).
All transitive managers were "derived" from
ClassicDependencyManagerthat was the default one in Maven 3. It operates (and later ignores) mgmt entries on quite "shallow" depths (depth >= 2). Hence, the way it was coded never caused an issue, as new instances are stopped being created quite early. OTOH, transitive operates all way down and observes all mgmt entries it finds on the way. and is hence, very memory hungry.Before this PR: each "derived" manager instance copied ALL contents from source (parent) manager, appended info as needed and stored. This causes huge memory allocation boom, as each manager instance holds (almost) same maps, as they are appended by level info if needed.
After this PR: manager instances keep track of their ancestors, and each manager keeps only "own" info, while the get/check now goes to root depMgr and descends to current. There are no more copies around of potentially huge maps. Also, removed redundant
Holderuse as well (kept it only on collection to memoize hash code), as it was redundant anyway and especially now, as "depth" is now meaningless: each entry is kept on own "depth" and not aggregated from root in huge maps with all entries from root to current node.