Page MenuHomePhabricator

Provide performance guidelines on when and how to load code
Open, Needs TriagePublic

Description

There are currently no guidelines on MediaWiki.org around how to load JS on the page.
I had a useful conversation with Timo today but would love to see this documented.

@Jdlrobson wrote:

Are there any performance guidelines on wiki which capture best practices of how to load JS code? e.g. something like the loading strategies section on https://phabricator.wikimedia.org/T235712 ?

[11:28 AM] @Krinkle wrote:
Jdlrobson: It's generally either addModules on page load, or mw.loader.using() to lazy load upon user interaction. There's no other strategies afaik needed currently.
these two are documeted at https://www.mediawiki.org/wiki/ResourceLoader/Developing_with_ResourceLoader#Loading_modules
How to detect MobileFrontend is imho out of scope for RL. That's part of core conventions for ExtensionRegistry (in PHP) and based on what promises/public stable interfaces MF wants to expose/maintain client-side.
Any kind of conditional is fine imho.

[11:30 AM] @Jdlrobson wrote:

Krinkle: not detection of MF
I was more interested in guidelines around WHEN to use mw.loader.using
For instance on Echo we load WHEN we CLICK the button not on page load
We are having a conversation now about how to architecture JS code in Vector so such a page would be super useful
ULS loads all its code on page load unlike Echo. Guidelines would help us migrate that to better loading strategies.
or maybe loading all code upfront is the guideline?
There is also talk about Vue.js - if we build out a new search interface with it, would we load the library up front or when the user focuses the search input?
Again a guidelines page with a performance team seal of approval would help steer this conversation greatly.

[11:33 AM] @Krinkle wrote:
Right, that's less about how to implement a certain direction, but what direction to pick overall.
When something is needed on page load, it's addModules(). There's only one way to do that really, and that's documented. To lazy-load, you use mw.loader.using().
I don't see how it ties in with desktop/mobile target stuff. Why are we looking to change how existing code is loaded as part of that?
I agree we can use better docs around that, and will try to find it and/or write it. In general, later is better. And loading Vue.js on page load is imho a no-go no matter what.
requestIdleCallback was a one-off experiment for Popups and shuld be removed, not recommended for any circumstance.
(There's a task about that)

[11:48 AM] @Jdlrobson wrote:

Is it okay for me to write a task requesting such docs?
Knowing performance guidelines on this would be useful. Vue.js being loaded on page load as already been mentioned several times.
Sorry for confusing by mentioning the targets system.
My reason for mentioning that was a lot of the blockers for enabling mobile target on modules is making sure they load where they are needed rather than unconditionally
A lot of extensions call addModules in OutputPage hook for example with no conditionals.
I suspect guidelines around what's expected would help that initiative too by making it crystal clear even though hopefully spelling out the obvious

[11:55 AM] @Krinkle wrote:
Yeah, I'll see what I can do, but ultimately it comes down to budgetting and performance review per-feature. I don't think we can describe clearly when it is fine to load on the page and when it isn't. In general, nothing should load on the page, unless it has to do something on that page and that it has to do it immediately when the page loads. That in itself is generally already something that should be avoided as it will not arrive for all users (on slow devices they can comfortably read a paragraph and move on to another page before it arrives), so whenever possible first try pure CSS-only etc. But if it has to be client-side, and has to be on page load, and doesn't result in bad UX, then loading the minimum needed on the pages right away is probably fine, but that still depends on where we are in our budget and how computationally expensive the code is, and how large it is.
(Feel free to ignore this, I'm just drafting out loud)
Things that fall under this is: "init" code that adds event handlers to the Edit button click event, to the Search input focus event. Code that makes the reserved space for sortable icons in tables filled in with an actual button and makes those buttons work. (Slight contradiction, why is it okay to not lazy-load jquery.tablesorter? This is a trade-off in size of the code, overhead of having yet another init module, and latency when the user clicks it given there is nothing to hide that latency behind, it is just sorting. Whereas search and edit do more things behind the scenes that already involve the network, so loading the code async is fine there. This is why the "init" code of VE isn't just a click handler, it also includes very minimal CSS to make that click result in the page looking like it is "loading".
also, I have a proposal to kill all "init" modules in favour of a standard attribute that will do this for you. E.g. you have an attribute in the HTML, when that is clicked, it will add a class that allows your CSS to make it look like however it should look when it is loading, and then when the code arrives it will call a certain method. The logic for this would be generic and only written once. See more at https://phabricator.wikimedia.org/T183720
This would mean we can remove almost all JS from page load, except for stuff that does non-user facing things (like navtiming metrics), and init code for which the loading state is very complex (more than a simple class name switch can do).
also, this will encourage use of "delegate event handlers". right now, a very common pattern (albeit a bad one) is to load JS that waits for dom-ready and then selects all of something and calls on(). This is problematic for many reasons. 1) It's poor UX because it means that when the JS has arrived, and most of the page is there and painted, the JS just sits there idle and the user can't click stuff. It waits for the whole page to be there below the fold, until 2) it selects all the relevent elements which is expensive and slow, and 3) allocates many event handlers, again expensive. Whereas if we use a delegate handler like $(document.body).on('click', '.my-stuff') you can run that right away without waiting for dom-ready, and allocates only 1 event handler, and any elements that exist now or in the future that match will just work, because we check it at click-time, instead of at select-time.
If I recall correctly, we adopted this in a few places last year around the Cite instrumentation that reading was involved with.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 27 2020, 10:27 PM
Jdlrobson renamed this task from Provide performance guidelines on when and how to load code to Provide performance guidelines on when and how to load code on mediawiki.org.Mar 27 2020, 10:27 PM
Demian updated the task description. (Show Details)Mar 28 2020, 7:54 PM
Demian added a subscriber: Krinkle.
Demian added a subscriber: Demian.

@Jdlrobson Thanks for documenting this. As irc logs aren't quite readable, I've formatted the discussion for phab.

Krinkle renamed this task from Provide performance guidelines on when and how to load code on mediawiki.org to Provide performance guidelines on when and how to load code.Mar 30 2020, 7:42 PM
nray awarded a token.May 13 2020, 8:59 PM
nray added a subscriber: nray.
nray added a comment.May 13 2020, 9:56 PM

Thanks for documenting this conversation @Jdlrobson @Krinkle and making it public! I found it pretty interesting particularly the part about the use of of event delegation which I've, perhaps incorrectly, always associated with having performance tradeoffs of its own. For example, on the jQuery API page for on it mentions:

Attaching many delegated event handlers near the top of the document tree can degrade performance. Each time the event occurs, jQuery must compare all selectors of all attached events of that type to every element in the path from the event target up to the top of the document. For best performance, attach delegated events at a document location as close as possible to the target elements. Avoid excessive use of document or document.body for delegated events on large documents.

I'm curious what practice we should employ as we progress with the Desktop Improvements project. Is the advised use of event delegation on wikis enabled by us having a relatively shallow (but wide) DOM tree so this path check is cheap and inconsequential compared to the delay associated with waiting for the DOM-ready event to fire before attaching events to relevant elements?

TheDJ added a subscriber: TheDJ.May 20 2020, 1:49 PM

There's only one way to do that really, and that's documented. To lazy-load, you use mw.loader.using().

Just a note that I think some people sometimes miss. addModules() and mw.loader.using() are not mutual exclusive and are not eachothers equals. addModules() and mw.loader.load() are the equals.

mw.loader.using() guarantees modules ARE loaded, BUT will wait for modules to load lazily if they are not yet loaded. The distinction is important. Code sometimes can have different loading patterns depending on state, entrypoint or priority, situations for which you don't want to completely write a separate JS module to follow a different load paradigm.

An example would be say a list, with an 'add' button. You can have a page with an empty list or a prefilled list. The JS module you ship for that page's list interactivity is one module (say A), but maybe the items within it require some sort of heavy JS dependency (say B) for their interactivity. You addModules() A always, but B conditionally on wether or not the list is empty. In A you have a clickhandler to add a new item to the list. That clickhandler would have mw.loader.using('B') and another promise with the dom building of the new listitem. When both promises finish, you run the setup of module B on the new item. When the original list already contained items, your mw.loader.using was basically a nullop, but if the list was empty it will have dynamically loaded. Same JS code, but different dependency shipping timing, dependent on initial state and guaranteed by mw.loader.using().

Note that often this is 'hyper optimization' which can be of questionable value/ROI. But if that page is a list of videos, with a software decoder library dependency and both empty lists and non-empty lists are rather common... it can be worth it.