Skip to content

Performance fix: lazier download of repodata#12

Merged
ethanholz merged 1 commit intoomsf:mainfrom
dwhswenson:fix-performance
Jun 18, 2025
Merged

Performance fix: lazier download of repodata#12
ethanholz merged 1 commit intoomsf:mainfrom
dwhswenson:fix-performance

Conversation

@dwhswenson
Copy link
Copy Markdown
Member

This pull request refactors the CondaReleaseSource class in src/spec0/releasesource.py to introduce a caching mechanism for repository data, enhancing performance by delaying the download until needed. The key changes include replacing the _repodata attribute with a cached property and updating related logic to utilize this cache.

Refactoring and Performance Improvements:

  • Introduced a _repodata_cache attribute and a _repodata property to manage repository data caching. This ensures that repository data is only fetched and processed once, improving efficiency.
  • Updated the logic for merging package data to use the _repodata_cache instead of directly modifying the _repodata attribute. This change aligns with the new caching mechanism.

@dwhswenson dwhswenson requested a review from Copilot June 12, 2025 21:17
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.30%. Comparing base (40a0bf7) to head (cc6a544).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #12   +/-   ##
=======================================
  Coverage   99.29%   99.30%           
=======================================
  Files          15       15           
  Lines         853      859    +6     
=======================================
+ Hits          847      853    +6     
  Misses          6        6           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors CondaReleaseSource to lazily fetch and cache repository data, improving performance by avoiding redundant downloads.

  • Added a _repodata_cache attribute and replaced direct _repodata assignment with a lazy-loaded @property.
  • Updated merge loops to populate the cache instead of modifying the attribute directly.
  • Maintained existing merge logic while deferring network and file operations until first access.
Comments suppressed due to low confidence (1)

src/spec0/releasesource.py:269

  • Add unit tests for the _repodata property to verify that repodata is fetched only once and that subsequent accesses return the cached result.
self._repodata_cache = None

self._channel_platforms = channel_platforms
self._repodata_cache = None

@property
Copy link

Copilot AI Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider using functools.cached_property instead of a manual cache to simplify the implementation and reduce boilerplate.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... I hadn't been aware of that. Nice trick to know, but I don't think I'll use it here. We might invalidate this cache at some point, and that's considerably uglier and less obvious with a functools.cached_property (you have to delete the entry from the instance's __dict__, as opposed to setting self._cache = None)

@dwhswenson dwhswenson requested a review from ethanholz June 12, 2025 21:34
Copy link
Copy Markdown
Contributor

@ethanholz ethanholz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM merging

@ethanholz ethanholz merged commit 2fbdcad into omsf:main Jun 18, 2025
6 checks passed
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.

3 participants