fix: introduce lock around deno info calls to fix OOM (#50)
#61
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fix builds on the insight in denoland#55 but does so by performing the minimal change required to fix the underlying issue. The reason for this departure from denoland#55 is due to our project's build failing when executed using the proof of concept's plugin, presumably due to some unknown change introduced on part of the proof of concept's extensive revision. The fix reuses @notcome's lock component and threads said lock through the codebase to ultimately place it around the `deno info --json ${id}` calls.
|
ping @marvinhagemeister: any thoughts on this fix for the plugin to enable its use by bounding the concurrency of Happy to iterate and make changes! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix builds on the insight in #55 but does so by performing the minimal change required to fix the underlying issue. The reason for this departure from #55 is due to our project's build failing when executed using the proof of concept's plugin, presumably due to some unknown change introduced on part of the proof of concept's extensive revision.
The fix reuses @notcome's lock component and threads said lock through the codebase to ultimately place it around the
deno info --json ${id}calls.When building our project using this patch of the plugin, my CPU utilization never goes about ~30% and the memory usage stays more or less constant. This is compare to before, where CPU utilization hit 100% and I ran out of memory and my system froze.
Fixes #50 and closes masslbs/Tennessine#330
For reviewers:
deno-vite-pluginitself i.e. MIT; @notcome could you let me know whether you license my usage of yourLockclass under MIT as well?