Performance enhancements #78
Comments
|
Forgot to mention, there are some caveats to the approach above:
HTH. |
yes createLink(mainScript);
yes for (let i = chunkSripts.length - 1; i >= 0; --i) {
const x = chunkSripts[i];
if (x.parentElement && mainScript.parentNode) {
x.parentElement.removeChild(x);
createLink(x);
}
}
yes stylesheets.forEach(link => {
noscriptTag.appendChild(link.cloneNode(false));
link.setAttribute("rel", "preload");
link.setAttribute("as", "style");
link.setAttribute("react-snap-onload", "this.rel='stylesheet'");
document.head.appendChild(link);
});
...
const scriptTag = document.createElement("script");
scriptTag.text = preloadPolyfill;
document.body.appendChild(scriptTag);
I know. I measured it too
Yes you are right, but this depends. You also can preload based on user activity. Or if you are using service workers all components will be preload eventually without additional code (assuming CRA setup). |
|
As of "preload on user activity":
I probably need to write more about performance optimisations, if it is not obvious from the readme. Thanks for feedback PS. if you are performance geek another thing on your list should be |
|
Thanks for the details, @stereobooster. My bad - I obviously didn't see your docs, and didn't see these optimisations by default in the examples I ran.
Any plans to modify this to preload and then insert the script instead, such that it doesn't block page load, maybe behind a config flag? |
there were no explicit docs about it
It is already in the bottom of document, all dom already is in place. I'm not sure what it can block. Can you show example which will clearly show the issue here. There is |
|
Sorry for the late reply.
(a) It is at the bottom of the page, so its request is delayed the most, (b) it isn't parallelised with other downloads, but is instead downloaded solitary in the end. (c) Because the app can't start up until this JS is downloaded (serially and in the end), the app takes longer to start. (d) window.onload isn't fired until after it is downloaded, which makes the browser loading indicators keep spinning, making the perceived page load time seem long.
When it's the last and only remaining thing to be downloaded, |
Can you show example of the application, which have this issue. Look I can craft example myself, but not necessary I will catch the issue you are describing. Without example this is purely theoretical talk, like in #72. I wasn't able to reproduce issue described in #72. |
|
Is there any part of my explanation above that makes it "purely theoretical talk"? To replicate, take a stock CRA app, run react-snap with all optimisations, note page load time is bound by markup+script, then run snapshotify, note that page load time with snapshotify is only bound by markup download, and not markup+script. I've already written an entire explanation of what the issue is, and how I've resolved it. Are you suggesting that I also build an entire real-word app to demonstrate this as well before you consider it worthy of investigation? |
I will take this as example. I will deploy it to firebase, and will run tests with webpagetest.org with moto g4 and 3G medium. Does this sound good? |
Or you could save some effort and see if my explanation above makes sense first. |
|
I see flaws in your explanations, but I do not want to keep building this tower of theoretical dispute if I can measure it ones. This is science, not theology - experimental data is the final verdict. |
Thanks for the detailed explanation.
Thanks for the on-point comparison.
Go ahead, then. You know how to replicate the issue. Use all your webpagetest and firebase and what not to get experimental data. I've spent enough time on this to discuss comparisons with theology. I already have working code that does all these optimisations. I'm only here suggesting that you incorporate these changes, so that my code doesn't need to exist. If you are going to be hostile about the whole thing, well, my code already works, and you've just made sure that my code continues to exist. If you think setting up experiments is easier than thinking through the problem logically, feel free to go ahead and make sure you're satisfied with all the experimental data you can collect. |
|
I'm sorry for the "theology" bit. I didn't mean to offense you, it was rather my frustration about theoretical disputes in general. This is inappropriate in this conversion I recognize it. On the other side you can not judge me for being sceptical. We both got to the same conclusion in purely theoretical dispute about #72. Experiment proved we both were wrong. My point let's skip useless dispute and jump straight to experiment. We can get into dispute about results of experiment.
Fair enough. Thanks for discovering issue with CSS-in-JS. |
|
To be clear, (a) #72 is still an issue I face, and (b) as confirmed by the author of loadable-components, there's an issue with component registration. You know both these facts. It's not a 'theoretical dispute', because it's neither theoretical, not disputed. It's a genuine bug, just one you don't face in your examples.
I am - or certainly was - hopeful that we could fix other such issues, so that we don't need to have separate projects. Ultimately though, this is your repo, and you can choose what to do with suggestions you get. If a (IMO) completely logical explanation is not going to be discussed, because 'theoretical' stuff is 'useless dispute', that's totally your call to make. |
|
I do not refer your problems as theoretical or non existent. I refer to your assumptions on why bug exist as theoretical (because assumption without proof is a theory).
But this is another issue, it has nothing to do to theory about next tick. This is issue of
No I don't know those facts, because your last response was
Completely logical explanation of thing that you can see but I can not? Let's us get into equal positions, so we both can see the issue and we can talk after. I want to dig to the root cause behind issue you are facing, but I can not do much until I can reproduce issue. I generally do not know if the issue is in |
|
Here is expiremnetal data https://github.com/stereobooster/test-snapshotify. From the given experiment I do not see any performance win of "onload" approach. |
|
I think we can both agree that this conversation is past its point of being useful. It's past the point where we're trying to understand and work on an optimisation. Instead, it seems to have devolved into trying to find ways to find fault with each other's arguments. I think we can both agree that that's not productive. I have to share some responsibility too in how we got here, and I'm genuinely sorry about that. Cheers! |
I know that
react-snapis primarily about creating static HTML snapshots, but there's a lot of opportunity to make performance tweaks to the built page. It would be awesome if these tweaks could be considered:window onloadevent. Until this event has fired, all browser indicators are firing to show that page load is in progress - things like the spinner in the tab and the message at the bottom of the window. Removing the usually large JS files from this critical page load process reduces the "apparent" page load time drastically. The solution is to use<link rel='preload' as='script' href='...' />in the<head>. This (a) kicks off script download early, (b) makes the script load process async and (c) doesn't hold back the window.onload event. (While investigating/testing for this, I suggest using the 'Slow 3G' setting in chrome devtools to throttle the network.)main.hash.jsin CRA apps) has to be downloaded and executed first, which then issues requests for the chunks (which then issues requests for other chunks, etc.). Downloading of all the needed chunks is hence a sequential process. This makes the page load process very time consuming. We already know during the snapshot process that the page needs additional chunks. So, we couldrel='preload'all of them upfront. This not only gets out of the way of the page load process, it actually significantly speeds up the time to initialise, since all the chunks are downloaded in parallel.<link rel='preload' ...downloads assets, it doesn't execute them, of course. I've handled this by injecting a "script loader" into the page, which inserts the main script file (main.hash.js) as a regular script tag after all the link-preloads have completed.glamorin lieu of external CSS files, but it's an easy win, so why not. Stylesheets also block the page load process. Since critical CSS is inlined during the snapshot process, again, there's no reason to hold back the page load event for external stylesheets. The solution again is to<link rel='preload' as='style' href='...' />the stylesheets, and inject a script that inserts this as a stylesheet after page load is complete. It's a very interesting trick too - inwindow.onload, simply change therel='preload'torel='stylesheet', and you're done. This one needs a fallback for users that have disabled JS in their browsers, so a<noscript>containing the regular<link rel='stylesheet' href='...' />in the head is required. Pages will take longer to load for people who have JS disabled, but at least things won't be outright broken for them.Just to give you an idea of how much savings this gives, on a site I'm working on (~160kb in assets for a page, mostly JS files), when I use the 'Slow 3G' throttling, before these changes, I was getting to DOMContentLoaded in ~6 seconds and Load in ~20 seconds. With these tweaks in place, I get to DOMContentLoaded in <2 seconds, and Load in <6 seconds. So, the savings aren't minor at all.
If you want to look at some example code, here I'm preloading scripts, and here I'm preloading stylesheets. I'm concatenating multiple script stubs together to inject into the page (so that there's only one script tag for easy CSP). Here's the stub to insert the script tag and here's the stub to insert the stylesheet. These stubs are combined and wrapped in a
window.on('load', function() { ... }), so that it's only executed after page load is completed.If this enhancement is mainlined, and #76 and #74 are resolved, and once I figure out what's going wrong with my
loadable-component, I can actually dump my repo and use react-snap instead. I createdsnapshotifybecause I felt like these changes are out of the scope ofreact-snap. However, if you think that these changes are not out of scope, then it doesn't make sense to split our efforts. I'll gladly contribute here instead. However, at the moment, it just felt like it'll be faster for me to create my own snapshot tool, and I'll probably stick to this decision for the immediate future. In the long term, it makes no sense to crowd the already crowded tooling space. :)Sorry for the long read, and thanks for entertaining the ramblings of a webperf nerd. Feel free to ask if you have questions. I may not be able to reply immediately, since I may have very spotty network coverage for the next few days.
The text was updated successfully, but these errors were encountered: