perf: build browser bundles at build-time #922

KuznetsovRoman · 2024-05-08T12:54:06Z

What is done

Build 2 browser bundles at build-time, instead of building them in runtime.
Moved 3 dependencies to devDeps, as they are not used anywhere else.
clientBridge.build now only reads one of these two bundles, without obligation to bundle them.

Benchmark

Setup

browser: local chrome
protocol: devtools
plugins: html-reporter, test-repeater (60 retries, when turned on)
isolation: disabled
test: empty

Before

First test

2500ms per test body on average

Screenshot

Other tests

200ms per test body on average

Screenshot

After

First test

1900ms per test body on average

Screenshot

Other tests

40ms per test body on average

Screenshot

KuznetsovRoman · 2024-05-08T12:55:33Z

package.json

-    "build": "tsc --build && npm run copy-static && npm run build-bundle -- --minify",
-    "build-bundle": "esbuild ./src/bundle/index.ts --outdir=./build/src/bundle --bundle --format=cjs --platform=node --target=ES2021",
-    "copy-static": "copyfiles 'src/browser/client-scripts/*' build",
+    "build": "tsc --build && npm run build-bundles",


Removed "copy-static" build, replaced "npm run build-bundle" with "npm run build-bundles".
"npm run build-bundles" also builds browser bundle

KuznetsovRoman · 2024-05-08T12:56:56Z

package.json

-    "copy-static": "copyfiles 'src/browser/client-scripts/*' build",
+    "build": "tsc --build && npm run build-bundles",
+    "build-node-bundle": "esbuild ./src/bundle/index.ts --outdir=./build/src/bundle --bundle --format=cjs --platform=node --target=ES2021",
+    "build-browser-bundle": "node ./src/browser/client-scripts/build.js",


I also tried to use browserify CLI, but it does not seem to support nested objects as transform params

KuznetsovRoman · 2024-05-08T12:58:13Z

src/browser/client-bridge/index.js


-    const lib = opts.calibration && opts.calibration.needsCompatLib ? "./lib.compat.js" : "./lib.native.js";
-    const ignoreAreas = opts.supportDeprecated ? "./ignore-areas.deprecated.js" : "./ignore-areas.js";


opts.supportDeprecated seems like a dead code. Haven't found any mentions about "supportDeprecated" anywhere else

KuznetsovRoman · 2024-05-08T12:58:45Z

src/browser/client-bridge/index.js

-        "uglifyify",
-    );
+    const scriptFileName = needsCompatLib ? "bundle.compat.js" : "bundle.js";
+    const scriptFilePath = path.join(__dirname, "..", "client-scripts", scriptFileName);


Read already-bundled code instead of bundling it here

KuznetsovRoman · 2024-05-08T13:03:34Z

test/src/browser/client-scripts/build.ts

+        assert.calledWith(writeFileStub, path.join(targetDir, "bundle.compat.js"), sinon.match(Buffer));
+    };
+
+    it("should build bundles for compat and native library", async function () {


I use one test for two cases because "browserify" is slow, and i dont want to stub it.

KuznetsovRoman · 2024-05-08T14:48:54Z

package-lock.json

The diff consists of marking nested deps as "dev only" and some @types packages

KuznetsovRoman · 2024-05-08T14:50:03Z

src/browser/calibrator/calibrate-client-script.js

Moved this one out of "client-scripts" folder, as now "client-scripts" don't exist in "build" ("build" directory would only have "bundle.js" and "bundle.compat.js" files)

But this one is needed for "calibrator", and it is not a part of client-scripts bundle

KuznetsovRoman · 2024-05-08T14:51:22Z

src/browser/client-bridge/index.js

-
-        return ClientBridge.create(browser, scripts);
-    });
+exports.build = async (browser, opts = {}) => {


I wanted to rewrite this file to typescript, but a lot of testing files depend on ability to mock this "build" method and we won't be able to do that with esm exports

KuznetsovRoman · 2024-05-08T14:51:50Z

src/browser/client-scripts/build.js

+/**
+ * @param {object} opts
+ * @param {boolean} opts.needsCompatLib
+ * @returns {Promise<Buffer>}
+ */
+const bundleScript = async opts => {


I am using plain JS with JSDoc instead of ts-node because ts-node is very slow, and it takes twice as more time to launch than running this script itself:

❯ time ./node_modules/.bin/ts-node -e "" ./node_modules/.bin/ts-node -e "" 3.76s user 0.26s system 143% cpu 2.803 total

❯ time node src/browser/client-scripts/build.js node src/browser/client-scripts/build.js 1.51s user 0.12s system 128% cpu 1.271 total

KuznetsovRoman · 2024-05-08T14:52:21Z

src/browser/client-scripts/ignore-areas.deprecated.js

As i haven't found any chances we could get "opts.supportDeprecated" truthy value inside of clientBridge.build calls, it seems to be the dead code

sipayRT · 2024-05-13T13:50:26Z

src/browser/calibrator/index.js

 const DIRECTION = { FORWARD: "forward", REVERSE: "reverse" };

 module.exports = class Calibrator {
    constructor() {
        this._cache = {};
+        this._script = fs.readFileSync(path.join(__dirname, "calibrate-client-script.js"), "utf8");


why did you change the path to the calibrate script? it would be better to keep all client scripts in the client-scripts directory

sipayRT · 2024-05-16T15:16:28Z

src/browser/client-scripts/build.js

+const bundleScript = async opts => {
+    const lib = opts.needsCompatLib ? "./lib.compat.js" : "./lib.native.js";
+    const script = browserify({
+        entries: "./scripts/index.js",


client-scripts/scripts/ doesn't look very nice (scripts/scripts). Maybe it would be better to rename with build or bundle

sipayRT · 2024-05-16T15:31:48Z

src/browser/client-scripts/build.js

+    await Promise.all(
+        [
+            { needsCompatLib: true, fileName: "bundle.compat.js" },
+            { needsCompatLib: false, fileName: "bundle.js" }


it would be more clearer to save compat/native naming

KuznetsovRoman force-pushed the TESTPLANE-16.browserify branch from 1788c64 to 6b657c1 Compare May 8, 2024 13:04

KuznetsovRoman commented May 8, 2024

View reviewed changes

KuznetsovRoman force-pushed the TESTPLANE-16.browserify branch 4 times, most recently from 815fc7e to 4adf8a3 Compare May 8, 2024 14:47

KuznetsovRoman commented May 8, 2024

View reviewed changes

perf: build browser bundles at build-time

37abf5f

KuznetsovRoman force-pushed the TESTPLANE-16.browserify branch from 4adf8a3 to 37abf5f Compare May 8, 2024 14:59

sipayRT approved these changes May 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: build browser bundles at build-time #922

perf: build browser bundles at build-time #922

KuznetsovRoman commented May 8, 2024 •

edited

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024 •

edited

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

KuznetsovRoman May 8, 2024

sipayRT May 13, 2024

sipayRT May 16, 2024

sipayRT May 16, 2024


		const lib = opts.calibration && opts.calibration.needsCompatLib ? "./lib.compat.js" : "./lib.native.js";
		const ignoreAreas = opts.supportDeprecated ? "./ignore-areas.deprecated.js" : "./ignore-areas.js";

perf: build browser bundles at build-time #922

Are you sure you want to change the base?

perf: build browser bundles at build-time #922

Conversation

KuznetsovRoman commented May 8, 2024 • edited

What is done

Benchmark

Setup

Before

First test

Other tests

After

First test

Other tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KuznetsovRoman May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KuznetsovRoman commented May 8, 2024 •

edited

KuznetsovRoman May 8, 2024 •

edited