Page MenuHomePhabricator

Split and lazy-load initially not needed ResourceLoader modules
Closed, ResolvedPublic3 Estimated Story Points

Description

Motivation
The loading time of the advanced search form got better:

But there is still room for improvement. On the live site the original search field shows up very fast, but it takes 2 or more seconds before the rest of the advanced search form shows up, even with our fast office network.
This ticket is about actually lazy-loading the code that is exclusively needed by the "advanced parameters" panel.

Acceptance Criteria

Notes:

  • Define a browser and connection speed for measurements
  • Measure the size and amount of time for the resources to be loaded
  • Consider giving visual feedback regarding loading status

Event Timeline

thiemowmde triaged this task as Medium priority.
thiemowmde moved this task from Backlog to Tickets ready for pickup on the Advanced-Search board.
kai.nissen set the point value for this task to 5.
• Lea_WMDE changed the point value for this task from 5 to 3.

Change 460915 had a related patch set uploaded (by Tonina Zhelyazkova; owner: Tonina Zhelyazkova):
[mediawiki/extensions/AdvancedSearch@master] Split resource modules

https://gerrit.wikimedia.org/r/460915

Change 461387 had a related patch set uploaded (by Gabriel Birke; owner: Gabriel Birke):
[mediawiki/extensions/AdvancedSearch@master] Lazy-load dependent module

https://gerrit.wikimedia.org/r/461387

This ticket description is very confusing to me, so I'll try to clarify the background and "acceptance criteria".

If I understand correctly, what we're trying to do is to split the initialization of AdvancedSearch into two parts: 1) Base widgets (search preview, namespace selection) 2) search widgets in the expandable pane.

There are at least two data points you can measure in JavaScript performance:

  • Transmission time, measuring the code size and the time it takes to be transmitted from server to client over a connection with a defined speed. You can measure the transmission time with and without "Time-to-first-byte" (TTFB), i.e. the times it takes the server to generate the compressed and minified JavaScript.
  • Execution time, measuring how long it takes to execute the transmitted JavaScript code (as a whole or until certain elements are visible).

Improving transmission time helps to deal with slow connections, improving execution time is for slow devices. There are other factors and data points like "parse time", the time it takes the browser between receiving the transmitted code and preparing it for execution, but we'll ignore those for now.

The current master branch is already optimized for execution time: It uses mw.requestIdleCallback to delay the initialization of the search field widgets inside the expandable pane, making the execution of the code for drawing the unexpanded AdvancedSearch faster.

That means this ticket is only about splitting the transmitted code into two parts, hopefully making the transmission time for the code for the unexpanded AdvancedSearch faster. I've done that with my PR, building on the patches by @Tonina_Zhelyazkova_WMDE and inspired by the PR by @thiemowmde .

The outcome of my patch is that the minified, uncompressed JavaScript is 22KB smaller. The remaining code (22 KB) is loaded with an additional HTTP request. I can't enable compression in my local environment, but if I look at the numbers at the article "Gzip Your Minified JavaScript Files" I'd guess that our 22KB will come down to about 10KB.
Until someone other than me can reliably turn on compression in the local environment, we can't measure the (micro)seconds saved on different connection speeds, we can only roughly calculate them. I'd advise removing the time-measurement from the AC, since setting up the local environment will involve lots of effort.

In case you're wondering if the rule of thumb "minimize HTTP requests" does not contradict the optimization made in this patch: I've found this article: https://mathiasbynens.be/notes/inline-vs-separate-file . Summary: Yes, HTTP requests add overhead (additional bytes) and loading time (TTFB, latency, TCP connection setup), but if your file is bigger than approx. 4-10KB then splitting the code is an improvement.

Currently, my patch will randomly fail browser tests, because the test system is not loading the search fields fast enough. I'll have to add "wait-for" statements to many of the browser tests for that :-(

The current PRs split one module into two module, which, as I see it, is not what @Krinkle meant. At the moment, we have over 35 "modules", each one with its own dependencies and most of them with only 1 script file. If I understand the comment correctly, we should only have 2-3 modules, each one with all the necessary script files. This might help ResourceLoader to generate the JavaScript faster (it has to do less work to calculate the "dependency graph" of the modules), maybe cache it better and reduce the overhead (shorter HTTP URLs -> shorter HTTP request/response headers). But I think this optimization should be split off into a different ticket, @Lea_WMDE

Change 460915 merged by jenkins-bot:
[mediawiki/extensions/AdvancedSearch@master] Split resource modules

https://gerrit.wikimedia.org/r/460915

Change 461387 merged by jenkins-bot:
[mediawiki/extensions/AdvancedSearch@master] Lazy-load dependent module

https://gerrit.wikimedia.org/r/461387

There is an other performance metric, and that is how long the user sees this:

Screenshot from 2018-09-24 11:57:37.png (772×323 px, 9 KB)

The problem here is one of perceived performance: Technically, the search field can be used instantly. The user does not need to wait. But the user does not know this. The animated loading indicator sucks the users attention and makes it look like the entire special page including the already visible input field can not be used until the loading finished.

Personally, I can imagine a few ways to make this less of a problem:

  • Doing what this ticket here suggests makes the loading animation disappear faster, because it needs to wait for less code to be loaded.
  • Move the animation down, as far away from the input field as possible.
  • Remove the animation entirely. Just show the empty white space.

Poking @Hanna_Petruschat_WMDE from WMDE-Design, maybe she can give her opinion if this is worth investigating further.

I response to @thiemowmde I am going to push a proposed fix which should address his concerns appropriately. I am in favor of keeping the dots because the amount of whitespace is just too much in my opinion and users may be confused why there is such a large gap between the search and search results.

It should also be noted that the position of the dots being so close to the input field is actually a bug in combination with custom CSS on the German Wikipedia. I have changed the CSS in way that fixes the issue on Wikipedia without introducing any side effects on other MediaWiki instances or Wikipedia users (fingers crossed 🤞).

On top of that, I have added a fade in for the dots using a pure CSS animation approach, the dots will now only appear after 1.1 seconds which and fully fade in within 400ms. This way, only users who are truly experiencing (network) performance issues should be seeing the dots (and thus increase the perceived performance of AdvancedSearch) without loading more bloating JavaScript to handle this case.

Here is a quick showcase of what it would now look like for users with a slow connection:

Users with a fast connection and under expected performance circumstances will not see the dots at all.