Incredibly slow performance

[Pauan]

🐛 bug report

Description of the problem

Hello, I am speaking of behalf of AmCharts.

We have hundreds of examples and tutorials on Codepen, but we would really like to move them over to CodeSandbox, because it is far superior in almost every way (great job, by the way!)

However, there is one major blocker which is preventing us from doing so, and that is performance.

Here is a sample Sandbox: https://codesandbox.io/s/4wnn4qwrvw

Wait for it to load (this takes a while). You will know it is working if you see a horizontal tapered chart.

After it's loaded, click the circular Reload button:

• On Chrome this reload takes ~1 second, which is acceptable.

• On Firefox this reload takes ~12 seconds, which is obviously far too long!

• It doesn't even work at all on Edge (it gives CORS network errors).

I tested on Windows 10 64-bit, with the latest Windows updates installed, and the latest versions of all the browsers.

We know that it is not a problem with our code, because the same Codepen loads fast, without any problems, in all browsers.

The only difference between the Sandbox and the Codepen is that the Sandbox is using ES6 modules whereas the Codepen is using <script> tags.

I tried creating a Sandbox that uses <script> tags, but that gives some bizarre Webpack errors (the same <script> tags load fine in Codepen). So it seems that Sandboxes manipulate the runtime environment in weird ways.

We also noticed that the transpilation takes a long time. That's okay if it's only done once and then cached on the server, but the transpilation seems to happen multiple times, even if the code hasn't changed!

If everybody who visits the Sandbox needs to re-transpile it, then that's also unacceptable for us (we need the Sandbox to load in ~1-2 seconds at most), so transpilation must be done only once (assuming no code changes).

[CompuIves]

Hey! Really cool to hear that you're considering using CodeSandbox for your examples! Thanks for letting us know about the performance issues, I just tried running the sandbox and I noticed the same thing. I think I pinpointed the issues though, I wrote a report on what causes the slowness and what we could do to solve it.

It's mostly because we weren't able to do many optimizations in this case, I can explain them and how we can enable some optimizations.

What makes it slow

Looking at the amchart example there are 3 imports at the start:

import * as am4core from "@amcharts/amcharts4/core";
import * as am4charts from "@amcharts/amcharts4/charts";
import am4themes_animated from "@amcharts/amcharts4/themes/animated";

The first two imports are what makes the execution quite slow. When we resolve those two files we download https://unpkg.com/@amcharts/amcharts4@4.0.0-beta.63/core.js and https://unpkg.com/@amcharts/amcharts4@4.0.0-beta.63/charts.js. We then transpile these files (because we see export/import statements which aren't supported in the browser) and download/transpile all the files that are referenced by those. This takes a long while and takes the main amount of time.

Normally we do some optimizations for this:

1. We prepackage any files we can find for a dependency on a server and cache that.
2. We don't transpile files if we detect that it's not needed.
3. We cache transpilation results on our server and in the browser.

I can explain them:

Prepackaging files

When we install a dependency we first preprocess them on a server, we check the main field in the package.json and download all files that are mentioned from the entry point and include them in the bundle. We also precompute the dependency graph for those files so we can skip that in the browser as well.

In this case we weren't able to do that since @amcharts/amcharts4 doesn't specify an entry point, this makes sense to me since there are multiple. That is the reason that we have to download all the files mentioned in the two files.

Transpilation skipping

When we detect a file that doesn't have any import/export statement and doesn't use new syntax we skip transpilation. We do a regex query to resolve the require statements from the file and skip sending the file to our transpilation workers. This makes transpilation very fast for those files (as we kind of skip it).

In this case the files seem to be transpiled, but there are still export/import statements, which forces the compiler to fall back to full transpilation to commonjs.

Transpilation caching

We cache the result of transpilation (dependency graph, transpilations) in IndexedDB and on our server in Redis. We have a limit of 7MB for the server cache per sandbox though, normally we never exceed this limit as most transpilation caches hover between 50kb and 200kb. In the case of this example the cache is 12MB. The reason for that is that we have to include all dynamically downloaded files/transpilations in the cache, otherwise we can't run it again. In the mentioned sandbox almost all files are dynamically downloaded since we weren't able to precompute the dependency graph and include those files in the dependency bundle.

Solutions

I think there are multiple easy ways to make the sandbox much faster.

Include a UMD build in the dependency and alias that in browser field

This should make the sandbox example as fast as Codepen. If you include a UMD bundle for core and charts and add them as aliases to the browser field in package.json we will use those files instead and instead of downloading/transpiling so many files we would only download and execute a single file.

Include a CommonJS version of the files

Another solution would be to ship the esmodules and commonjs versions separately. If we detect a commonjs version we won't have to retranspile them, which will reduce the cache size and make transpilation faster.

Separate the dependency in 2 packages

This is the most drastic change, and I understand that it doesn't make sense to do this just to make it run faster in CodeSandbox. One way to make the bundle fast in CodeSandbox would be to separate charts and core in their own dependencies, and let main resolve to the index.js of those dependencies. That way CodeSandbox can pre-optimize the dependency by precomputing the dependency tree and including all files in the initial bundle. It would put the cache size around ~50kb.

Use the script tags like CodePen

I found out why the sandbox with the script tag didn't work. It seems like the script tag was using window.webpackJsonP for dynamically loading some files and we use that global as well for dynamically downloading, haha. I changed our global name to be more unique, the example should work now.

This one is a valid solution to make it work, but you'd lose things like type definitions.

Solutions I can do

I think the biggest improvement I can do is find ways to optimize the cache to make it lower than 7MB. I think we could probably make this lower by compressing the cache before we send it, I will try playing with that. I will also see if we can increase our Redis storage and potentially double our limit. Maybe there's a way to not save the downloaded files in the cache or only save the most important downloaded files, like a filter.

I will also look at what happens on Edge, that sounds like some weird issues!

Thanks for letting us know again of the slowness, this is really helpful and will also help us in future cases. I'll see what I can do to optimize it to make it as fast as an instant load.

[Pauan]

@CompuIves Thank you very much for the very long (and fast!) response!

I've spent some time reading and thinking about what you said, and I generally understand the situation. I'm very glad to hear that CodeSandbox has extensive caching.

However, I have some more questions:

• Why can't it precompute the dependency graph for npm packages which don't have a main?

  It should be able to look at every .js file, and if it hasn't been seen yet it will add it to the graph, and then it will recurse into each dependency. Here's some pseudo-code:

  var seen = {};
  var graph = makeGraph();
  
  function computeGraph(files) {
      files.forEach(function (file) {
          if (file.extension === ".js") {
              if (!seen[file.absolutePath]) {
                  seen[file.absolutePath] = true;
  
                  graph.addToGraphSomehow(file);
  
                  var dependencies = parse(file.absolutePath).getDependencies();
                  computeGraph(dependencies);
              }
          }
      });
  }
  
  computeGraph(getAllFilesForPackage());

  That should allow it to create a full dependency graph in linear time (with respect to the number of .js files).

• Why does it work well on Chrome, but is so super slow on Firefox? If the issue is that it's not being cached, shouldn't that affect all browsers?

• When we compile an AmCharts demo locally (using Webpack) the index.js file is 758 KB.

  It also has various other dynamically loaded files (some of which are .map files for source maps, and some of which are .js files imported using the import(...) feature). They total 12.6 MB.

  That particular demo doesn't load any of the dynamic files, so the total download size should be 758 KB.

  I understand the cache has to store everything (which puts it above 7 MB), but if it can precompute the graph, it should be able to split the static and dynamic dependencies, so that way it knows the static dependencies are only 758 KB (and the dynamic dependencies are 12.6 MB).

  In other words, you could have multiple levels of caching: a small fast cache for stuff which is always needed (like index.js), and a slower big cache for things which are dynamically loaded (like .map files, or files loaded with import(...))

  Does that sound reasonable, or are there some issues with implementing that?

• In this case the files seem to be transpiled, but there are still export/import statements

  Yes, it is standard practice to ship transpiled files which contain export/import, because then downstream users get tree shaking (tree shaking doesn't work with CommonJS).

  which forces the compiler to fall back to full transpilation to commonjs.

  Since it is best practice to ship pre-transpiled files with export/import, perhaps it could detect that and avoid transpiling in that specific case?

  Since export/import are natively handled by Webpack/Parcel/Rollup, there's no need for transpilation if export/import are the only ES6 features which are being used.

• It would put the cache size around ~50kb.

  How is that possible? Even at the smallest size, a transpiled demo with AmCharts will be at least ~700 KB.

[Pauan]

Also, I just now tested the <script> version of our demo, it works great in Chrome (loads very fast), but doesn't work at all in Firefox or Edge. Is that also happening for you, or is it a stale cache issue?

[CompuIves]

Why can't it precompute the dependency graph for npm packages which don't have a main?

We precompute the dependency graph for a single entry point, determines from main. We could look at multiple entry points and precompute a graph for that, but that can become very big. I haven't thought of partial dependency graphs per file though, and that does sound super interesting! Then we could kind of stitch the partial dependency graphs to one big one for the entry point. I will look into that.

Note that it wouldn't help a lot with performance in this case by the way, since we still need to generate an AST to transpile the import/export properly. Most of the time is put into generating an AST.

Why does it work well on Chrome, but is so super slow on Firefox? If the issue is that it's not being cached, shouldn't that affect all browsers?

I'll look into this. The numbers indeed look very weird, maybe there is a problem with resolving the cache on Firefox or we use a function that's optimized in Chrome but isn't in Firefox.

When we compile an AmCharts demo locally (using Webpack) the index.js file is 758 KB.

I think the main reason for the low size from Webpack is that Webpack does tree shaking (I think it only includes the imports that are used from core/charts). We currently haven't implemented this, although I was planning on adding it in the future. Tree shaking takes a lot of computing power and more memory (we suddenly need to keep track of export/imports in files as well) and takes a while to implement, so my reasoning was that it was okay to download a bit more for faster run speed. But in this case it seems that tree shaking is much more beneficial and we should start using it. I believe most bundlers don't tree shake in development mode too, but they have all the files available anyway!

Do you have an example of dynamic imports? We could absolutely split those already from the cache, that seems like a nice optimization!

Since it is best practice to ship pre-transpiled files with export/import, perhaps it could detect that and avoid transpiling in that specific case?

export/import is invalid syntax in many browsers currently. If we don't transpile those files we will get a syntax error from the browsers.

Since export/import are natively handled by Webpack/Parcel/Rollup, there's no need for transpilation if export/import are the only ES6 features which are being used.

Webpack/Parcel/Rollup still generate an AST for every file to read export/import + convert them to a require implementation. Generating AST takes most time in the transpilation. Which makes me think, a nice optimization we could do in the packager is detecting import/export and generate an AST on the server already. It would make the bundle size a bit bigger though.

How is that possible? Even at the smallest size, a transpiled demo with AmCharts will be at least ~700 KB.

In this scenario we can reuse the precomputations of the dependency graph. This means that we only need to save the transpiled code of the files created by the user. This is really cool, the default React sandbox has only a cache of 12kb for example.

Also, I just now tested the <script> version of our demo, it works great in Chrome (loads very fast), but doesn't work at all in Firefox or Edge. Is that also happening for you, or is it a stale cache issue?

We use document.write in the Parcel template. I recently found out that the Firefox/Edge implementation of document.write is not done according to spec so I'll have to disable it for everything except for Chrome. I will do that now!

[Pauan]

Do you have an example of dynamic imports

Here are some places inside our library which contain dynamic imports:

https://github.com/amcharts/amcharts4/blob/aa37ef9baf11d43a4884dedfe2ad680575cdeecb/dist/es2015/.internal/core/export/Export.js#L2524

https://github.com/amcharts/amcharts4/blob/aa37ef9baf11d43a4884dedfe2ad680575cdeecb/dist/es2015/.internal/core/export/Export.js#L2559-L2560

https://github.com/amcharts/amcharts4/blob/aa37ef9baf11d43a4884dedfe2ad680575cdeecb/dist/es2015/.internal/core/export/Export.js#L2602

All of our dynamic loading is done with the import(...) expression, and they all use static paths like import("./path/to/foo") or import("foo"), so they shouldn't be hard to analyze and split from the rest of the bundle.

By default Webpack and Parcel will automatically split the dynamically loaded files into separate chunks. That's why index.js is only 758 KB, rather than 13.4 MB, since it doesn't include any of the dynamically loaded files.

If we don't transpile those files we will get a syntax error from the browsers.

Sure, but the built-in support for export/import in bundlers should still be a lot faster than running a full transpile (like Babel).

Webpack/Parcel/Rollup still generate an AST for every file to read export/import + convert them to a require implementation. Generating AST takes most time in the transpilation.

Of course having fast transpilation would be good, but I'm not too concerned about that (as long as it's done once and cached on the server). It's the amount of time to load the Sandbox that really concerns us.

In this scenario we can reuse the precomputations of the dependency graph. This means that we only need to save the transpiled code of the files created by the user. This is really cool, the default React sandbox has only a cache of 12kb for example.

That is very cool!

We use document.write in the Parcel template. I recently found out that the Firefox/Edge implementation of document.write is not done according to spec so I'll have to disable it for everything except for Chrome.

The <script> version now works in Firefox, but it seems to do a weird "double load", where it will start loading the page and then when it's half-way done it reloads it. It's quite noticeable and jarring.

Why is it using document.write at all? Shouldn't it just be writing into an <iframe>? Or perhaps using Blob:

var blob = new Blob([sandboxSourceCode], { type: "text/html" });
var url = URL.createObjectURL(blob);
iframe.src = url;

[mbochynski]

I have the same performance issue for my almost vanilla codesandbox
All code that I've added was:

it("should be true", () => {
  expect(true).toBe(true);
});

The test suite run forever on Firefox and everything works smooth on Chrome.