The Wayback Machine - https://web.archive.org/web/20230223200953/https://github.com/microsoft/playwright/pull/20287
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

[WIP] Code-split components when component testing #20287

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pastelsky
Copy link
Contributor

@pastelsky pastelsky commented Jan 23, 2023

Code splitting ensures that the test JS / CSS is the minimal required for a test to run in isolation, which should ease up memory and CPU when executing tests.

Code splitting also introduces script boundaries, which means that a runtime error in the global code of another component would not lead to the entire test suite blowing up.

This comes at a price of a slight increase in bundling time if your module graph is large and you have a lot of components, but this should be offset by the fact that once compilation happens, running individual tests is a lot faster.

When running with an without code-splitting on a large real-world test suite at Atlassian with 900+ tests, I noticed that the average time per test dropped from ~5 seconds / test to < 2 seconds / test when code-splitting. Browsers also consumed less memory and CPU which meant we could push for better parallelization on the same hardware.

  • Code split for React
  • Code split for Vue
  • Code split for Vue 2
  • Code split for Solid
  • Code split for Svelte
  • Test all implementations manually / Add additional tests
@pastelsky pastelsky changed the title Code-split components when component testing [WIP] Code-split components when component testing Jan 23, 2023
@pavelfeldman
Copy link
Member

Could you help me understand where exactly are the performance gains coming from? I.e. what used to happen and is no longer happening.

@pastelsky
Copy link
Contributor Author

pastelsky commented Jan 23, 2023 via email

@pavelfeldman
Copy link
Member

I guess I'm trying to understand, which line gives bundler a hint on whether it should split the code and whether we can rely on that. You are still importing everything unconditionally as per my read of the code.

In terms of (1), (2) and (3), one would need to stress the system very hard in order to see the effect in (1) and (2). And even for (3), given that component testing is reusing the browser context, everything should settle in the disk and compile caches of the browser, so you should not see any amortized savings. Of course doing less work is always better, but I'm curious why the average test time has changed - maybe we are under-utilizing the browser's compilation cache.

@pastelsky
Copy link
Contributor Author

pastelsky commented Jan 24, 2023

I guess I'm trying to understand, which line gives bundler a hint on whether it should split the code and whether we can rely on that.

Dynamic imports establish code split points. See —
https://github.com/microsoft/playwright/pull/20287/files#diff-322969de049b8aa51c59cf00d1461d810e49833f396f39a6bd3e03d734a98919L306-L313

(In terms of (1), (2) and (3), one would need to stress the system very hard in order to see the effect in (1) and (2)

Yes, in our case, we are talking about a large app, with the monolithic bundle produced standing at ~8MB. With a concurrency of ~10 browsers on an M1 mac (say), the webserver needs to be streaming ~80MB at a time (worst case), in which case the performance of the webserver etc. does start to matter. Most apps using playwright might not be large enough for this, but should still see smaller improvements.

And even for (3), given that component testing is reusing the browser context, everything should settle in the disk and compile caches of the browser.

This is interesting because it seems in our case that the webserver essentially serves the bundle for each test case (we logged requests served). If there was disk caching/compile caching involved, we wouldn't be seeing this, nor would we pay the parse cost again. That is unexpected according to you?

Lastly, the isolation benefits (which aligns one of playwright's core philosophies) of code split entry points is a reason on its own — even if we discount the performance benefits.

@pavelfeldman
Copy link
Member

Dynamic imports establish code split points.

I see, but it looks like all modules are loaded unconditionally, or why is it not happening? Every time index.js is loaded, you still load all the bundles, where is the performance win coming from?

@pastelsky
Copy link
Contributor Author

pastelsky commented Jan 29, 2023

@pavelfeldman Ah, I can see why that might have been confusing. Our internal implementation for code splitting was implemented in userland. I missed making the dynamic imports lazy while copying over changes in this PR to the vitePlugin.

They should be lazily loaded and evaluated on a need-basis now.

for (const [alias, value] of componentRegistry) {
const importPath = value.isModuleOrAlias ? value.importPath : './' + path.relative(folder, value.importPath).replace(/\\/g, '/');
if (value.importedName)
lines.push(`const ${alias} = () => import('${importPath}').then((mod) => mod.${value.importedName});`);
else
lines.push(`const ${alias} = () => import('${importPath}');`);

@pastelsky pastelsky force-pushed the sk/code-split-components branch 2 times, most recently from c827bf1 to 30c7d3e Compare January 29, 2023 15:58
@pastelsky
Copy link
Contributor Author

I'll take a look at a few of the failing tests this week. But would be good to know if I'm right directionally here.

@pavelfeldman
Copy link
Member

Yes, with the latest changes that turn the import into the arrow function, I understand how it becomes lazy / module split!

await fs.promises.writeFile(buildInfoFile, JSON.stringify(buildInfo, undefined, 2));

for (const [filename, testInfo] of Object.entries(buildInfo.tests))
setExternalDependencies(filename, testInfo.deps);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's what is making the tests fail.

Shubham Kanodia added 3 commits February 19, 2023 12:20
Code splitting ensures that the test JS / CSS is the minimal required for a test to run in isolation, which should ease up memory and CPU when executing tests. Code splitting also introduces script boundaries, which means that a runtime error in code of another component would not mean that the whole test suite blows up. This comes at a price of a slight increase in bundling time if your module graph is large and you have a lot of components, but this should be offset by the fact that once compilation happens, running individual tests are a lot faster in such cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants