-
-
Notifications
You must be signed in to change notification settings - Fork 821
Defer non-critical JS for slightly faster page load under CPU+Network simultaneous bottleneck #4798
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
Conversation
Interesting! I mean, under good conditions you won't notice a difference. But there doesn't seem to be any risk in adding ChatGPT's risk assessment sounds promissing:
Ready to merge? Or are you still testing? |
@falkoschindler I believe this is ready to merge. My brain was just stuck in the thinking mode of #4756 for no reason 😅 and thinking that I need to init the Socket.IO ASAP. But, before that PR gets merged, Socket.IO isn't initiated until we run It has a very very tiny risk of breaking existing code, if the user uses ui.add_body_html('''
<script>
console.log(Vue); // breaks after this PR
</script>
''') But I don't think we should be held back by that very niche use case. |
Before I forget: If you really need to use Vue in document.addEventListener('DOMContentLoaded', function() {
// Your code here
}); |
…ad (CPU-bound) (#4801) ### Motivation First, I discovered the `nomodule` attribute of `script` completely by accident in #4761. I pitched that it can speed up page load, but never got around to testing it. I was going on a trip back then. I promptly forgot about it. 😅 Until #4798 where we discovered the importance of applying a CPU throttle to make speed issues show themselves, and I was reminded of the topic, and proceeded to implement it and benchmark it. I found a significant speed increase. ### Implementation Simply set `nomodule` to the ES module shim. ### Progress - [x] I chose a meaningful title that completes the sentence: "If applied, this PR will..." - [x] The implementation is complete. - [x] Pytests have been added (or are not necessary). - [x] Documentation has been added (or is not necessary). ### Results showcase: Low-tier mobile speed throttling (8.5x on my machine), no need disable cache and network throttling (we want to show **only** JS execution time) Before:  After:  Finish: 7.71s -> 6.97s (Difference: -0.74s, Percentage Difference: -9.60%) DOMContentLoaded: 6.48s -> 5.85s (Difference: -0.63s, Percentage Difference: -9.72%) Load: 7.57s -> 6.82s (Difference: -0.75s, Percentage Difference: -9.91%) ### Potential impacts We can no longer use the functionalities that ES Module Shims offer to us - `importShim.getImportMap()` - `importShim.addImportMap(importMap);` - Shim import `importShim('/path/to/module.js').then(x => console.log(x));` - Dynamically injecting import map like ```js document.head.appendChild(Object.assign(document.createElement('script'), { type: 'importmap', innerHTML: JSON.stringify({ imports: { x: './y.js' } }), })); ``` I doubt they are useful, since if your browser doesn't support ES Module, ES Module Shims does **not** magically make NiceGUI run with better compatibility. Notably, `ui.markdown` with Mermaid diagram always breaks.
Motivation
Still in the mood of speed improvement.
So I was reading https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/script again...
Basically, loading behaviour wise, there's 3 valid options:
(Don't think about defer async. It has been mentioned that if async is set, "If the attribute is specified with the defer attribute, the element will act as if only the async attribute is specified", meaning async + defer => async).
I then took a look at the script situation in NiceGUI:
<script type="module">
, which is "defer by default"Then I thought: Can I have the rest of the NiceGUI's scripts be defer as well, since they need to run before the main script only, and not any earlier. We don't need to block up the parser while those scripts load?
And yes, indeed, there is performance uplift.
Implementation
defer
would make it even faster, or it would break things. I'll see when I have time.Progress
-> No need pytest and documentation. As long as existing tests don't break, Id be happy
Results showcase
Using Fast 4G profile, 4x CPU slowdown.
Before:
After:
Finish: 6.47s -> 5.99s
DOMContentLoaded: 4.69s -> 4.38s
Load: 5.69s -> 5.18s
About save 5s.
Reference: 4G network latency 165ms. I am not saying that we are saving 3x the network latency, but it just puts things into perspective.
Worthy of note:
May want to test with more slowdown and Slow 4G later. 3G's too much.