Skip to content

Commit

Permalink
fix: spawning too many esbuilds builds (#491)
Browse files Browse the repository at this point in the history
* Fix spawning multiple esbuilds on same file, that would cause memory usage to spike.
According to documentation of the changes in esbuild 0.17, calling rebuild on the same build params is “safe” in the sense that it would merge it into an existing build rather then spawn a new one.

Furthermore, the contexes of these rebuilds are now needed to be disposed of. That was the original issue that made me use build over the rebuild - in the initial import of esbuild 19, running serverless invoke and serverless package would never finish. Using build seemingly solved this, but it ended up causing the issue spawning too many builds described above.

I added a step on finishing invoking, deploying and packaging to make sure to dispose of the contexes.

* Use build if there is no context

* Proper naming
  • Loading branch information
ZachLeviPixel committed Sep 20, 2023
1 parent 2eb287a commit 36ef92e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
13 changes: 9 additions & 4 deletions src/bundle.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import assert from 'assert';
import { build } from 'esbuild';
import type { BuildOptions } from 'esbuild';
import type { BuildOptions, BuildResult } from 'esbuild';
import fs from 'fs-extra';
import pMap from 'p-map';
import path from 'path';
Expand Down Expand Up @@ -101,8 +100,14 @@ export async function bundle(this: EsbuildServerlessPlugin): Promise<void> {
outdir: path.join(buildDirPath, path.dirname(entry)),
};

let context!: BuildContext;
const result = await build(options);
const pkg = await import('esbuild');
const context: BuildContext = await pkg.context(options);
let result!: BuildResult;
if (context) {
result = await context.rebuild();
} else {
result = await pkg.build(options);
}

if (config.metafile) {
fs.writeFileSync(
Expand Down
13 changes: 13 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class EsbuildServerlessPlugin implements ServerlessPlugin {
},
'after:package:createDeploymentArtifacts': async () => {
this.log.verbose('after:package:createDeploymentArtifacts');
await this.disposeContexts();
await this.cleanup();
},
'before:deploy:function:packageFunction': async () => {
Expand All @@ -170,6 +171,7 @@ class EsbuildServerlessPlugin implements ServerlessPlugin {
},
'after:deploy:function:packageFunction': async () => {
this.log.verbose('after:deploy:function:packageFunction');
await this.disposeContexts();
await this.cleanup();
},
'before:invoke:local:invoke': async () => {
Expand All @@ -179,6 +181,9 @@ class EsbuildServerlessPlugin implements ServerlessPlugin {
await this.copyExtras();
await this.preLocal();
},
'after:invoke:local:invoke': async () => {
await this.disposeContexts();
},
};
}

Expand Down Expand Up @@ -510,6 +515,14 @@ class EsbuildServerlessPlugin implements ServerlessPlugin {
service.package.artifact = path.join(SERVERLESS_FOLDER, path.basename(service.package.artifact));
}

async disposeContexts(): Promise<void> {
for (const { context } of Object.values(this.buildCache)) {
if (context) {
await context.dispose();
}
}
}

async cleanup(): Promise<void> {
await this.moveArtifacts();

Expand Down

0 comments on commit 36ef92e

Please sign in to comment.