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

Check for existing results to rebuild #123

Merged
merged 2 commits into from
May 5, 2021

Conversation

anthony-zhou
Copy link
Contributor

Currently, this plugin calls the ESBuild build method each time a file is changed.

As documented in #98, this behavior results in exponentially increasing memory usage as files are changed. Personally, I saw my memory usage (from the esbuild process) spike from an initial value of 600 MB to 2GB after just 5 changes. Ultimately, the memory usage results in a fatal error, where the JavaScript heap runs out of memory.

What I did

In this pull request, I've added a conditional check in the bundle method to call rebuild instead of build if there are existing build results.

I also set the bundle method to enable incremental builds by default. From what I understand, the way the ESBuild process works is as documented here:

  1. Create an initial build with incremental: true.
  2. Rebuild as necessary by calling rebuild on the existing build result.

Results

With the change in this PR, memory usage increases much more slowly. After about 10 file changes, the memory only increased to about 1GB (I noticed a linear increase of about 30MB each time). Further debugging is needed to determine the source of that slight memory usage increase, but it seems like this optimization addresses the majority of the existing increases in memory usage.

@floydspace
Copy link
Owner

Hi @anthony-zhou , appreciate your contribution and memory analyze. I see we have already similar PR opened #101, could you pls check it and make a new measurement with your test case? so we can kill 2 PRs if it successful.

@anthony-zhou
Copy link
Contributor Author

anthony-zhou commented May 5, 2021

@floydspace After trying #101, the memory usage there is the same as before -- it increases exponentially (~3GB memory usage after 6 changes). I think the reason is that, in #101, the initial build is not created with the incremental flag set to true, meaning the rebuild method is never defined (as you mentioned in a comment on that PR). As such, the rebuild method never gets called.

@floydspace
Copy link
Owner

@anthony-zhou okay merging it. Looks fine for me

@floydspace floydspace merged commit 457e513 into floydspace:master May 5, 2021
@github-actions
Copy link

github-actions bot commented May 5, 2021

🎉 This PR is included in version 1.10.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants