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: add maxMemoryLimitBeforeRecycle options #58

Merged
merged 1 commit into from Jun 27, 2023

Conversation

AriPerkkio
Copy link
Member

The implementation is highly inspired by jest-worker's idleMemoryLimit option.

Each worker reports their process.memoryUsage().heapUsed to main thread when they have finished the current task. Main thread compares this limit to pool's new memory limit option and decides whether worker should be terminated and replaced with new one.

@AriPerkkio
Copy link
Member Author

@sheremet-va any thoughts? Maybe you could test this against vitest-dev/vitest#3203 before we merge and release this?

@Aslemammad
Copy link
Member

@AriPerkkio Any updates on this PR?

@AriPerkkio
Copy link
Member Author

No updates from my side. I think this should work as is but maybe let's first test this with Vitest's work-in-progress vm isolation.

@sheremet-va
Copy link
Member

No updates from my side. I think this should work as is but maybe let's first test this with Vitest's work-in-progress vm isolation.

I did not check it yet, VM support is not on my list yet, but will be in a few weeks. I was planning to release a sort of "road map" where everyone can see open PRs and help. Feel free to test this with an open VM PR yourself and make changes if you need to.

@sheremet-va
Copy link
Member

Looks like we can merge this and release a new version.

@Aslemammad Aslemammad merged commit 28ba15f into tinylibs:main Jun 27, 2023
6 checks passed
@AriPerkkio AriPerkkio deleted the feat/memory-limit-recycle branch June 27, 2023 12:18
@AriPerkkio
Copy link
Member Author

Testing this with Vitest and seeing crashes when final round of tests are run and workers are recycled. I think this condition needs some changes:

if (!this.options.isolateWorkers) pool.workers.maybeAvailable(workerInfo)

Looking into this.

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.

Add a way to recycle workers when heap limit is reached
3 participants