Skip to content

fix(issue-634): reduce memory leak #686

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Jun 27, 2025

Fixes #634

Note: there is a memory footprint reduction, there are some ideas how to optimise in follow-up PR

  • added benchmark tests, and created a fixture to instantiate a large chart (copy files)
  • added Rules, to easy maintain add/remote file patterns
  • created an issue Function loader.Load() potential leaking resources helm/helm#31007
  • added LoaderWithRules, helm itself not sure if required such capability, but required optimisation for helm-unittest, as we executing dozens of helm.Load operations, while helm <command> only runs it once

I have tried deep_copy https://github.com/helm-unittest/helm-unittest/blob/main/pkg/unittest/deep_copy.go, it does not work very well,

Following approach has it's own pros/cons

From the pros side

  • we could maintain our set of rules for Load operation to improve resource consumption, like include/exclude values files or snapshots for each test
  • in follow-up we could easily add caching, so instead of evaluating rules on each for idx, testJob := range s.Tests , we could do them ouside of a loop and pass only files required for current test.

cons:

  • loose coupling with helm

CPU reduction 4 times
Memory reduced in 2.5x times

Before
Screenshot 2025-06-27 at 14 53 58

With the fix
Screenshot 2025-06-27 at 15 01 33

At some point, depends on where or not issue is accepted, I could try to submit few PRs to helm, but the process there is quite lengthy.

@ivankatliarchuk ivankatliarchuk changed the title fix-634: resolve memory leak fix(issue-634): resolve memory leak Jun 27, 2025
@ivankatliarchuk
Copy link
Contributor Author

Hi @quintush. When you have time, pls have a look and share what you think.

@ivankatliarchuk ivankatliarchuk force-pushed the fix-634-reduce-memory-leak branch from 9a4686a to 98f0a28 Compare June 27, 2025 14:50
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

fix-634: resolve memory leak

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

fix-634: resolve memory leak

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
@ivankatliarchuk ivankatliarchuk force-pushed the fix-634-reduce-memory-leak branch from 98f0a28 to faa4876 Compare June 27, 2025 15:16
@ivankatliarchuk ivankatliarchuk changed the title fix(issue-634): resolve memory leak fix(issue-634): reduce memory leak Jun 28, 2025
@bavaria-blue
Copy link

This is a great project. Thanks @ivankatliarchuk for the PR.
We would like to upgrade to v1.0.0, but are blocked by the memory leak, which this PR would remedy.
Just a reminder - would be great @quintush if you could find the time to look at this. Thanks!

@teimyBr
Copy link

teimyBr commented Aug 4, 2025

Would be great to have a 1.0.1 Version soon with this PR :D

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.

Potential memory leak in version 0.8.1 causing high resource usage
3 participants