Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance for transitive dependency checks #1381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

To6i
Copy link

@To6i To6i commented Nov 19, 2024

TransitiveDependencyCondition internally calls contains() recursively on the collection of all objects to be tested. If this collection is a large list and there are enough recursive calls to getDirectDependencyTargetsOutsideOfAnalyzedClasses() this results in a heavy performance impact. On a reasonable large project a single test using that condition may take minutes to complete.

Here is a 30 seconds FlameGraph taken while an transitive check was running for > 2 minutes:

FlameGraph_30s

Based on the samples, the CPU hangs in this filter lamdba for > 86% of the time:

  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (47,247,323 samples, 0.03%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (44,447,761 samples, 0.03%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (10,837,882,731 samples, 7.32%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (13,262,127,759 samples, 8.96%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (12,668,650,362 samples, 8.56%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (15,368,403,186 samples, 10.38%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (46,224,364 samples, 0.03%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (47,314,101 samples, 0.03%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (18,048,208,277 samples, 12.19%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (13,405,921,387 samples, 9.06%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (93,152,524 samples, 0.06%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (3,244,023,882 samples, 2.19%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (919,865,902 samples, 0.62%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (6,438,577,874 samples, 4.35%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (6,760,263,856 samples, 4.57%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (7,031,313,250 samples, 4.75%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (7,723,048,585 samples, 5.22%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (11,918,050,716 samples, 8.05%)
  • TransitiveDependencyCondition$$Lambda$553.0x000000080121ca88:::test (46,026,822 samples, 0.03%)

So, converting the given list to a Set with much better contains() performance fixes this issue.

@To6i To6i force-pushed the improve-performance-transitivedependencycondition branch 3 times, most recently from a8aff4f to 852e52f Compare November 20, 2024 09:46
`TransitiveDependencyCondition` internally calls `contains()` recursively
on the collection of all objects to be tested. If this collection is a
large list and there are enough recursive calls to
`getDirectDependencyTargetsOutsideOfAnalyzedClasses()` this results in a
heavy performance impact. On a reasonable large project a single test
using that condition may take minutes to complete. Converting the given
list to a Set with much better `contains()` performance fixes this issue.

on-behalf-of: @e-solutions-GmbH <[email protected]>
Signed-off-by: To6i <[email protected]>
@To6i To6i force-pushed the improve-performance-transitivedependencycondition branch from 852e52f to 54a4c9d Compare November 20, 2024 09:52
@To6i To6i marked this pull request as ready for review November 20, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant