Skip to content

Cache and reuse resolution results#3931

Merged
jeremylong merged 5 commits intomainfrom
issue-3923
Jan 5, 2022
Merged

Cache and reuse resolution results#3931
jeremylong merged 5 commits intomainfrom
issue-3923

Conversation

@aikebah
Copy link
Copy Markdown
Collaborator

@aikebah aikebah commented Dec 30, 2021

Description of Change

Cache resolution results for all dependencies and reuse that instead of doing (un)filtered resolutions dependency-by-dependency. Fixes #3923

Have test cases been added to cover the new functionality?

no, functionality was refactored

@boring-cyborg boring-cyborg Bot added the maven changes to the maven plugin label Dec 30, 2021
@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Dec 31, 2021

@jeremylong a good step forward. But definitely needs more work as it currently creates duplicate dependencies on aggregate-scanning a large multimodule project

@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Jan 2, 2022

@jeremylong Differences in results can be explained by a failure to resolve the reactor-projects in the new code plus existing behaviour that virtual dependencies are not merge together as a single one.

If you're working towards a 6.5.2 release feel free to already promote this from draft to normal and merge it. I will try to find out why the resolution did work in the 6.5.1 case and doesn't work in the 6.5.2-SNAPSHOT case from this branch, but the difference I spotted is in my view sufficiently explainable to warrant inclusion of this change in a next fixpack in order to solve the massive slowdown.

The project I used had 10 inter-module-dependencies, which were 10 resolved dependencies in 6.5.1. In 6.5.2-SNAPSHOT they become unresolved, triggering virtual dependency creation. And as some of those inter-module dependencies occur for multiple dependencies it results in a total of 24 inter-module dependencies being listed for what actually is only 10 inter-module dependencies.

That multiplication of virtual dependencies also happens in 6.5.1 if a reactor-dependency in a project is not yet resolvable.

Not resolving the (existing) release versions of the reactor-dependencies I consider a regression of this change. Duplicating the virtual dependencies I will file a separate GH issue once I've managed to tackle this regression.
If you decide to merge this already in order to put out a 6.5.2 the regression could be handled under a new GH issue.

@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Jan 2, 2022

@jeremylong already found the pattern when it fails: when the artifact searched for uses a classifier. So think it's better to get this resolved before merging.

@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Jan 2, 2022

@jeremylong already found the pattern when it fails: when the artifact searched for uses a classifier. So think it's better to get this resolved before merging.

Minor correction, not the classifier, but a non-jar packaging (ejb)

@aikebah aikebah marked this pull request as ready for review January 2, 2022 16:22
@aikebah aikebah added this to the 6.5.2 milestone Jan 2, 2022
@aikebah aikebah changed the title Attempt fixing Issue 3923 by caching and reusing resolution results Cache and reuse resolution results Jan 2, 2022
@jeremylong
Copy link
Copy Markdown
Collaborator

I feel like we need to release 6.5.2. We can publish another release as soon as the regressions are solved.

@jeremylong jeremylong modified the milestones: 6.5.2, 6.5.3 Jan 3, 2022
@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Jan 3, 2022

I feel like we need to release 6.5.2. We can publish another release as soon as the regressions are solved.

Sorry I hadn't commented that the regression was resolved by my last commit. Could've gone into the 6.5.2. But I propose to hold back 6.5.3 until the duplication of virtual dependencies (#3944) also gets resolved as well as I intend to look at that tonight.

@aikebah
Copy link
Copy Markdown
Collaborator Author

aikebah commented Jan 3, 2022

@jeremylong Fix for #3944 is in its final testing stage and will get its own branch and PR

@jeremylong jeremylong merged commit 40630a1 into main Jan 5, 2022
@jeremylong jeremylong deleted the issue-3923 branch January 5, 2022 10:48
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

maven changes to the maven plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Huge performance degradation after upgrade dependency-check-maven from 6.3.1 to 6.4.x / 6.5.x

2 participants