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

feat(http): Stop caching GET responses in memory #33854

Closed

Conversation

zharinov
Copy link
Collaborator

@zharinov zharinov commented Jan 26, 2025

Changes

Remove memory cache for GET requests during repository run.

Previously, all GET requests were cached in memory during repository runs.
Since then, Renovate has evolved to use package/repository cache more efficiently.
The memory cache is now redundant and unnecessarily consumes resources,
as these requests are unlikely to be repeated during a single run.

To justify that, I created the test repo with package.json stolen from Renovate.

This is the heap dump in main branch after dry-run:

Google Chrome 2025-01-26 16 49 55

This is the heap dump performed by the branch of this PR:

Google Chrome 2025-01-26 16 50 15
  • Another example is Zed editor repo: 300Mb vs 200Mb (lots of Rust deps)
  • My Rubygems test repo: 350Mb vs 300Mb

Context

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@zharinov zharinov requested review from rarkins and viceice January 26, 2025 20:52
@rarkins
Copy link
Collaborator

rarkins commented Jan 26, 2025

Memory is reduced but need to confirm that http requests don't increase accordingly

@zharinov
Copy link
Collaborator Author

Yeah, I'll create "cache provider" class for memory cache too, and then will use it for all cases that rely on this type of caching (e.g. platform code).

@rarkins
Copy link
Collaborator

rarkins commented Jan 27, 2025

Before this?

@zharinov
Copy link
Collaborator Author

Yep, so I need the bunch of PRs to be merged

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

so marking this as draft

@viceice viceice marked this pull request as draft January 27, 2025 15:01
@zharinov
Copy link
Collaborator Author

@zharinov zharinov closed this Jan 28, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants