LeaVerou / css3test

How does your browser score for its CSS3 support?
http://css3test.com
MIT License
214 stars 83 forks source link

Concerns about loading ES modules #228

Open PolariTOON opened 2 years ago

PolariTOON commented 2 years ago

Issue #216 is about refactoring the projet to use modern JavaScript features. PR #227 was the first part towards this goal, by introducing the first ES modules. We started considering the impact on performance so i'm pasting some previous comments to move the discussion here.

@PolariTOON wrote in #219:

I agree splitting the tests into several files, possibly ES modules, would be useful to make maintainance easier. This would also heavily increase the number of requests made to load all the tests, but also to incrementally load them by importing them dynamically without waiting for the whole testbase to be fetched and parsed. So the impact on performance have to be considered.

@PolariTOON wrote in #227:

I understand re-exporting all the specs from one place (tests.js) is convenient for legibility but i think this is far from being ideal from a performance point of view, because this means these 105 (!) imports are actually discovered by the browser after two consecutive subrequests: csstest.js, and then tests.js.

The easier way i see to improve that is to import the specs directly from csstest.js. Or we could preload some (all?) the files with <link rel="modulepreload">, but the support for it is not great currently.

@LeaVerou wrote in #227:

Importing the same module twice does not fire a separate HTTP request. Also, isn't the whole point of HTTP/2 that multiple HTTP requests are fine? :smile:

@SebastianZ wrote in #227:

tests.js is just one additional request, so it doesn't make much of a change to remove it. I could remove it, though I kept it for readablility and to keep the Specs structure unchanged. [...] I agree we should add <link rel="modulepreload">s. In browsers that don't support it it's just ignored, so there's no harm in adding it. I'd add one for all tests except the CSS 2 ones because they are excluded by default. [...] I've tested that now in Chrome 97 and Firefox 96 and didn't see any performance gain. Therefore I do not add them. @PolariTOON If you think we can somehow get a performance improvement by preloading resources, I suggest you open a separate issue and we'll discuss it there.


Let me reply to some of these comments:

Importing the same module twice does not fire a separate HTTP request.

This is not about importing the same module twice, but about the depth of the request graph which now looks like this:

index.html
└── csstest.js
    └── tests.js
        └── tests/a.js
        └── tests/b.js
        └── ...

This means that the browser have to wait for csstest.js to be fetched and parsed to fetch tests.js, and then wait again for it to be parsed to fetch all the modules under tests/. To allow the browser to load the page faster, we have to let it discover the files earlier. This can be achieved by reducing the depth of the module graph -- for example, importing all the modules from the top-level module (csstest.js currently) -- or even better (in theory), by declaring them in the HTML code via <link rel="modulepreload">.

Also, isn't the whole point of HTTP/2 that multiple HTTP requests are fine? :smile:

Indeed, once the browser knows about the files under tests/, these ones can be fetched in parallel and it's fine with the help of HTTP/2.

I've tested that now in Chrome 97 and Firefox 96 and didn't see any performance gain.

As far as i know, only Blink browsers support module preloading currently so it's normal not to see any improvement in Firefox. As for Chrome, i guess the gain should only be visible if all the dependencies are preloaded. According to the waterfall below, this should save about 300ms, but this should not change the "Time taken" measurement on the page because it only counts the execution time and not the roundtrips between the client and the server.


Before #227: css3test-classic-performance After #227: css3test-module-performance

SebastianZ commented 2 years ago

To allow the browser to load the page faster, we have to let it discover the files earlier. This can be achieved by reducing the depth of the module graph -- for example, importing all the modules from the top-level module (csstest.js currently)

Importing the modules directly from csstest.js and with that removing tests.js would be ok for me.

Also, isn't the whole point of HTTP/2 that multiple HTTP requests are fine? 😄

Indeed, once the browser knows about the files under tests/, these ones can be fetched in parallel and it's fine with the help of HTTP/2.

To clarify things, HTTP/2 is about reusing the same connection for multiple requests. Though multiple requests still cause some overhead due to their headers in comparison to a single one.

I've tested that now in Chrome 97 and Firefox 96 and didn't see any performance gain.

As far as i know, only Blink browsers support module preloading currently

Right, for anyone wanting to follow implementations in other browser engines, see https://wkb.ug/180574 and https://bugzil.la/1425310.

As for Chrome, i guess the gain should only be visible if all the dependencies are preloaded. According to the waterfall below, this should save about 300ms, but this should not change the "Time taken" measurement on the page because it only counts the execution time and not the roundtrips between the client and the server.

I'll try it again and provide some numbers.

Sebastian