Prevent concurrent builds from happening #27
Merged
Conversation
This file contains 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
AMP noticed that multiple _initial_ builds can happen if Karma takes a long time between calls to karma-esbuild's preprocessor (in our case, we have hundreds of files having SHA hashes performed). This lead to multiple `bundle.write` calls happening. Now, our test suite is so large, esbuild takes a few seconds to run through everything. So when the second `bundle.write` happens, we're still trying to bundle the first time. This leads to 2 initial builds. Because both builds are `incremental`, they hold on to esbuild's Go process. And because we would lose our reference to the first incremental build when the second finished, we now had an undying holder on esbuild. While solving this concurrent initial build problem, I decided that we should never have 2 concurrent builds happening at the same time. Now only 1 build can be happening at a time, and if multiple write come in (either during the initial build or during a rebuild), we'll wait for the in-progress build to finish before starting the new one. Async reentrant problems are hard.
jridgewell
force-pushed
the
reinitial-bundle
branch
from
b83bb3c
to
dfc4067
Compare
Let the default equality message shine through
marvinhagemeister
approved these changes
Mar 20, 2021
samouri
reviewed
Mar 22, 2021
| // case, we resolve the old build with the latest result. | ||
| private buildsInProgress = 0; | ||
| // buildInProgress tracks the in-progress build. When a build takes too | ||
| // long, a new build may have been requsted before the original completed. |
| this.beforeProcess(); | ||
| } else { | ||
| // Wait for the previous build to happen before we continue. This prevents | ||
| // any reentrant behavior, and guarnatees we can get an initial bundle to |
There was a problem hiding this comment.
nit: s/guarnatees/guarantees
| @@ -100,7 +122,10 @@ export class Bundle { | |||
| // Wait for any in-progress builds to finish. At this point, we know no | |||
| // new ones will come in, we're just waiting for the current one to | |||
| // finish running. | |||
| if (this.buildsInProgress > 0 || this._dirty) { | |||
| if (this.buildInProgress || this._dirty) { | |||
| // Wait on the deffered, not the buildInProgress, because the dirty flag | |||
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.
AMP noticed that multiple initial builds can happen if Karma takes a long time between calls to karma-esbuild's preprocessor (in our case, we have hundreds of files having SHA hashes performed). This lead to multiple
bundle.writecalls happening.Now, our test suite is so large, esbuild takes a few seconds to run through everything. So when the second
bundle.writehappens, we're still trying to bundle the first time. This leads to 2 initial builds. Because both builds areincremental, they hold on to esbuild's Go process. And because we would lose our reference to the first incremental build when the second finished, we now had an undying holder on esbuild.While solving this concurrent initial build problem, I decided that we should never have 2 concurrent builds happening at the same time. Now only 1 build can be happening at a time, and if multiple write come in (either during the initial build or during a rebuild), we'll wait for the in-progress build to finish before starting the new one. A fun challenge was ensuring that we can only have at most 1 in-progress build and 1 pending build.
Async reentrant problems are hard.