resourceloader: Optimise resolved-check in sortDependencies()
authorTimo Tijhof <krinklemail@gmail.com>
Mon, 8 Apr 2019 00:45:48 +0000 (01:45 +0100)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 9 Apr 2019 07:06:28 +0000 (07:06 +0000)
commit78efe6963f3514ead5af9559d3472547d5924570
tree3215b9f0c31162ea2b04a736fc399109558c01df
parent5dfedaeaa42f13f115ae649ad5cfe319e6acc682
resourceloader: Optimise resolved-check in sortDependencies()

Remove the `resolved.indexOf` check from code outside the
for-loop because it's not needed for correctness. The function
is private, and its result is passed to enqueue() and work()
which already do their own de-duplication.

It was here as optimisation to keep the result fairly small
and avoid needless computation, but this isn't useful in practice
because the input never contains duplicates in production.
There are only two ways to call it:

1. From mw.loader.load(), which we never give duplicates.

2. Recursively from within sortDependencies() itself, which
   is already guarded by exactly the same check.

Move the handling of `baseModules` to the caller so that it
does not need to check it N times for every top-level module on
a page and N more times for every dependency of every dependency
of every such module etc. Instead, do it once before we start.

Also update error "Unknown dependency" to say "Unknown module",
because this function deals both with top-level modules and with
dependencies. In fact, most reports of this error in Phabricator
are for top-level modules, not dependencies.
This makes sense given dependency validation via PHPUnit,
which means such error would be unlikely in prod. An unknown
module, on the other hand, is quite normal (and caught) for
cases where a module or gadget is removed but still queued in
cached HTML.

Change-Id: I944593994a66f1c65c0732d7d1f2d60ed4226e79
resources/src/startup/mediawiki.js
tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js