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

fix: add cache for OrtDependency model objects to OrtModelBuilder #9818

Conversation

jjohannes
Copy link
Contributor

Only create one 'OrtDependency' for each 'ResolvedComponentResult'. The 'ResolvedComponentResult' is immutable and therefore all 'OrtDependency' object derived from the same 'ResolvedComponentResult' have the same content. Hence, we can reuse these objects to save memory and computation time.

Without this, large builds seem to run forever:
#9763

This PR is a refined version of the fix posted in the error description of #9763

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.07%. Comparing base (449e89c) to head (06baf39).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #9818   +/-   ##
=========================================
  Coverage     68.07%   68.07%           
- Complexity     1285     1287    +2     
=========================================
  Files           249      249           
  Lines          8828     8828           
  Branches        918      918           
=========================================
  Hits           6010     6010           
  Misses         2432     2432           
  Partials        386      386           
Flag Coverage Δ
funTest-docker 65.00% <ø> (ø)
funTest-non-docker 33.49% <ø> (+0.13%) ⬆️
test-ubuntu-24.04 35.90% <ø> (ø)
test-windows-2022 35.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sschuberth sschuberth force-pushed the gradle-model-object-deduplication branch from 60bbc2e to 95f5e0e Compare January 23, 2025 21:38
@sschuberth sschuberth marked this pull request as ready for review January 23, 2025 21:38
@sschuberth sschuberth requested a review from a team as a code owner January 23, 2025 21:38
@sschuberth sschuberth force-pushed the gradle-model-object-deduplication branch from 95f5e0e to 8c0e700 Compare January 23, 2025 21:41
@sschuberth sschuberth enabled auto-merge (rebase) January 23, 2025 21:44
Only create one 'OrtDependency' for each 'ResolvedComponentResult'.
The 'ResolvedComponentResult' is immutable and therefore all
'OrtDependency' object derived from the same 'ResolvedComponentResult'
have the same content. Hence, we can reuse these objects to save memory
and computation time.

This PR is a refined version of the fix posted in the error description
of [1].

[1]: oss-review-toolkit#9763

Signed-off-by: Jendrik Johannes <[email protected]>
@sschuberth sschuberth force-pushed the gradle-model-object-deduplication branch from 8c0e700 to 06baf39 Compare January 23, 2025 21:46
@sschuberth sschuberth merged commit 7c63104 into oss-review-toolkit:main Jan 23, 2025
25 checks passed
@jjohannes jjohannes deleted the gradle-model-object-deduplication branch February 4, 2025 13:45
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.

2 participants