@Ladsgroup After a conversation starting about ResourceLoader being the reason behind having icon packs with @matmarex earlier he shared the following insights:

14:37 <MatmaRex> yes. creating a hundred modules, one for each icon, would cause performance issues for all page loads
14:40 <MatmaRex> (for example, the SyntaxHighlight extension used to define a module for each supported language, and we eventually rewrote it: T85794)
14:41 <Volker_E> MatmaRex: so there's no useful path forward in our current setting to [load] just a few distinct icons depending on the context
14:43 <MatmaRex> i think back then, and more recently in T155813#3047635 , there have been ideas of allowing "parameterized" module names, which would effectively allow us to deliver the CSS needed for single icons without registering a billion modules
14:44 <MatmaRex> (looks like in T155813 they decided on another approach, so no one is working on actually implementing this. but it's probably possible to do)

Thanks for the notes, it explains a lot. I have an idea that might help: Let extensions define their own icon packs and icon styles using json. So extensions can have something like this https://github.com/wikimedia/mediawiki/blob/master/resources/lib/oojs-ui/themes/mediawiki/icons-content.json and mediawiki understands that. It can be really helpful since an extension might not want the variants and that can help in performance greatly.

This will add lots of other unneeded icons. For example see: https://gerrit.wikimedia.org/r/#/c/338215/2. Also see https://en.wikipedia.org/w/load.php?debug=true&lang=en&modules=oojs-ui.styles.icons%2Cicons-alerts&only=styles&skin=vector and compare it with one or two icons there

Why does the request contain both the base icons and icons-alerts? As far as I know there is no dependency between them. The base one is just another icon pack. Requesting instead https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=oojs-ui.styles.icons-alerts&only=styles&skin=vector reduces the request to 15 icons (2.9KB compressed). That seems quite reasonable.

The same patch also calls enableOOUI() which indeed results in other stuff happening and being loaded, which seems like the more significant problem (not the size of the icon pack).

Composing custom icon packs from extensions, using icons already defined in MediaWiki core, seems undesirable at first as it would result in multiple modules containing the same icons (e.g. ext.flaggedRevs.icons and oojs-ui.icons-alerts).

Another way would be to register each icon as its own module, which would likely cancel out any win that may've been gained from it.

While it may feel wrong to get more than you need, this is the acceptable reality of our current module loading. It's not very different from loading a JavaScript module and using only 6 out of 10 of a class' methods. While not "perfect", it's not a very big waste. As long as the module packs are kept to a reasonable size (for example 10-15 icons, < 5KB) I think the compromise is worth the benefit.

Other than that, I'd say don't worry about the performance in this case and use the infrastructure as-is if it works. If it becomes a problem, I'm always open to revisiting this, but for now I'd recommend closing this issue.

In T160690#3109332, @Ladsgroup wrote:

Thanks for the notes, it explains a lot. I have an idea that might help: Let extensions define their own icon packs and icon styles using json. So extensions can have something like this https://github.com/wikimedia/mediawiki/blob/master/resources/lib/oojs-ui/themes/mediawiki/icons-content.json and mediawiki understands that. It can be really helpful since an extension might not want the variants and that can help in performance greatly.

It is already possible. You can create a module with 'class' => 'ResourceLoaderImageModule' (or ResourceLoaderOOUIImageModule if you need different icons per theme). For example, Echo does it for the personal bar icons it needs on every page: https://github.com/wikimedia/mediawiki-extensions-Echo/blob/d2b93f5624c10b3226de154d516c2000138cdcce/extension.json#L521 + https://github.com/wikimedia/mediawiki-extensions-Echo/blob/d2b93f5624c10b3226de154d516c2000138cdcce/modules/icons/mediawiki/badgeicons.json (this example seems more complicated than it needs to be, since all themes use the same images, but it's okay - see also docs at https://www.mediawiki.org/wiki/ResourceLoader/Images).

Of course doing that is a bit contrary to the goal of "icon standardization", since your duplicated icons won't be updated - but it's an option if performance demands it. (This also allows extensions to define completely custom theme packs, e.g. VisualEditor does it.)

I applied what you mentioned in https://gerrit.wikimedia.org/r/#/c/338215/3. Thanks. For me, this card is still valid but probably lowest priority (or declined)

In T160690#3108906, @Volker_E wrote:

14:43 <MatmaRex> i think back then, and more recently in T155813#3047635 , there have been ideas of allowing "parameterized" module names, which would effectively allow us to deliver the CSS needed for single icons without registering a billion modules
14:44 <MatmaRex> (looks like in T155813 they decided on another approach, so no one is working on actually implementing this. but it's probably possible to do)

Suddenly there is a proposed patch to implement this in https://gerrit.wikimedia.org/r/347442.

@Krinkle MobileFrontend identified recently number of unused icons in icon packs as blocker for their OOUI adoption in T181108: Reconsider loading icon packs in enableOOUI for optimising critical rendering path as problematic rendering-critical payload. Although we can separate icons into icon packs, there might always be use cases where you need only a few icons out of three or four packs resulting in an unsatisfying or even unacceptable performance impact for mobile.
@Jdlrobson's reaction on lazy-loading icons would render several of the basic OOUI widgets unidentifiable/unusable in no-JS environment apart from problems around weird FOUC (FOII).
Can we provide a possible, performant way out here?

Once T177432 is done we'll have a set of icons defined in core. When this happens I think there's an opportunity to direct extensions/skins to start using these icons in core rather than icons local to the repo.

Instead of doing:

		"mobile.search.images": {
			"class": "ResourceLoaderImageModule",
			"selector": ".mw-ui-icon-{name}:before",
			"images": {
				"clear": "resources/mobile.search.images/clear.svg",
				"search-content": "resources/mobile.search.images/search-content.svg"
			}
		},

I would like to do:

		"mobile.search.images": {
			"class": "ResourceLoaderIconPackModule",
			"selector": ".mw-ui-icon-{name}:before",
			"images": [ "clear", "search-content" ]
		},

This would just be a wrapper class for a ResourceLoaderImage module where the icons are sourced from the known set of icons.

It may result in more icon packs then needed, for example in the original description FlaggedRevs would have the flexibility of introducing a new module with the single icon it needs:

		"FlaggedRevs.icon": {
			"class": "ResourceLoaderIconPackModule",
			"images": [ "circle" ]
		},

We may even want to consider making icons a part of ResourceLoaderFileModule, if we are worried about too many icon packs (FWIW I find it a pain having to create a separate resource loader for icons when I already have a module with JS and CSS.

So, with some folks considering T181108 a blocker to wider use of OOUI and some other folks considering the alternative solution of T166948 unacceptable, I suppose we're now also interested in pushing this through.

As I understand, there are two worries about allowing this:

• Each registered module must be listed in the "manifest" that we send to the client, which allows mw.loader.load() to load modules and handle cache invalidation (of the browser's built-in cache and of mw.loader.store). If we add (potentially) a couple hundreds of modules, that actually becomes a big chunk of data to load before literally everything else.
  
  Anomie's patch avoids this problem by only registering the single "wildcard" module (invalidating the cache for it invalidates it for all possible "wildcard" values).

• The result for each URL like load.php?modules=herp.derp.foo,bar,baz is cached server-side (in Varnish), so if it was possible to generate hundreds of such URLs from one module, that could allow the caches to grow too large.
  
  But as long as the "parameters" to the "wildcard" module are hardcoded (e.g. not generated from user input or such), and if we don't go too wild with separating out tiny sets of them (think grouping them… like in our icon packs), this should not be a problem.
  
  Keeping them grouped also should be better for performance in the long run: for example, it seems better to load one slightly bigger stylesheet (e.g. load.php?modules=icons.foo,bar,baz) on three different pages, than three slightly smaller stylesheets on each of them (e.g. load.php?modules=icons.foo,bar; load.php?modules=icons.foo,baz; load.php?modules=icons.bar,baz), since the browser caches the bigger one on the first load and can serve it immediately on the next two.

@Krinkle Is that mostly correct?

@Anomie's patch (https://gerrit.wikimedia.org/r/347442) seems to do this right. I haven't yet tried to write the glue code to use this mechanism for OOUI icon modules, but it should be possible.

Note that with this proposal, the icons to load would be listed in the addModuleStyles() call, rather than the module definition. To borrow @Jdlrobson's example above, it would become something like:

		"mobile.search.images.*": {
			"class": "ResourceLoaderIconPackWildcardModule",
			"selector": ".mw-ui-icon-{name}:before"
		},

$out->addModuleStyles( 'mobile.search.images.clear', 'mobile.search.images.search-content' );

Change 410038 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Allow loading styles for individual OOUI icons

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

Feels like a good topic to discuss in next frontend standards.

@Krinkle wrote at https://gerrit.wikimedia.org/r/347442 "resourceloader: Add wildcard modules":

---

This approach and implementation looks good to me. But as always with new features, I want to make sure that before we land this, that it has an existing use, and to evaluate whether this use's underlying needs can be accommodated in a clean way without the extra feature.

As I understand it, the use is already up, which is the loading of individual OOUI icons. And the use case is presumably to not need to maintain icon packs anymore. What that sounds great from a maintainer's perspective (less is more), I want to take some time to also evaluate this from a performance perspective because I do think there's a non-zero cost associated with specifying icons individually in that each module being loaded and stored comes with boilerplate and overhead.

Quite often in practice, I find that, while it feels counter-intuitive, it can sometimes be a net-win to load a (small amount of) unnecessary code if it means there's a higher likelihood of cache hits and cache re-use, this in turn comes with a smaller startup registry, fewer boilerplate wraps, and smaller localStorage footprint. Especially for small things like icons, it is not inconceivable that the boilerplate and overhead for this may be equal to the size of numerous icons.

As for the startup registry, having only 1 wildcard module in the registry instead of N icon packs or N icons, is a win in that we register fewer modules.

But, it is also a loss, because it means we register more dependencies - which are transferred in that same startup registry. Wildcards also mean that we have less perfect cache invalidation (retransfers more often than needed, and thus more bandwidth consumption).

All to say, that, this code is great, and I want to improve performance any way we can. I am just not currently of the belief that it obviously will improve performance. I believe it can, and that it is also possible that it might not. I'd like to run some numbers on this and see it in action, so that we can make an informed decision.

Also leaving the option open that if it turns out negative, that I will commit to helping find an alternative solution, which might involve re-embracing icon packs, and approving their use more widely (which I believe currently is perceived by some people as a reason to avoid icon packs - if so, it'd be great to have a list of those on a relevant Phab task so we can run the numbers against that as well).

I see that in the commit message at https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/410038/, @matmarex has already done some research as well. I'll be sure to look at that as well. Sorry for not noticing that before.

Change 410038 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Allow loading styles for individual OOUI icons

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

In T160690#4917902, @Krinkle wrote:

I see that in the commit message at https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/410038/, @matmarex has already done some research as well. I'll be sure to look at that as well. Sorry for not noticing that before.

Let's move it here, probably easier to discuss this way. The numbers below are from January but I see no reason why they would have changed significantly:

Impact of this change:

• On all pages: Size of the startup module increased by ~1.5 kB (size of the code from https://gerrit.wikimedia.org/r/c/mediawiki/core/+/347442/ and new module definitions from https://gerrit.wikimedia.org/r/c/mediawiki/core/+/410038/). :(

• On a page that only loads OOUI core (e.g. Special:Diff): Size of OOUI CSS decreased by ~4.5 kB. :)
• On a page that loads OOUI core and widgets (e.g. Special:MovePage): Size of OOUI CSS decreased by additional ~10 kB. :D

• On a page that loads many icon packs (UploadWizard): Size of icon pack CSS increased by ~14 kB.
• On a page that loads many icon packs (VisualEditor): Size of icon pack CSS increased by ~18 kB.
• Size increases due to the added overhead of mw.loader.implement calls for each icon and the new deprecation warnings. This can be resolved by updating the code to load individual icons. I'm pointing this out to note that the overhead is not too insane even when we're unnecessarily loading many extra modules, and the improvement from updating the dependencies of OOUI modules themselves [T181108: -4.5 kB and -10 kB, as mentioned above] already almost nullifies it.

(Values after gzip. Exact numbers will depend on how many other modules are loaded in the same request, which affects compression. Tested on a wiki with a bunch of random extensions installed.)

Change 510994 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Allow loading styles for arbitrary OOUI icon packs

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

I heard through the grapevine that the proposed solution is not to everyone's liking. Perhaps this alternative will be easier to stomach. It ditches all the magic with wildcard modules and just allows defining custom icon packs.

Change 510994 merged by jenkins-bot:
[mediawiki/core@master] Allow loading styles for arbitrary OOUI icon packs

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

Change 410038 abandoned by Bartosz Dziewoński:
Allow loading styles for individual OOUI icons

Reason:
Abandoning in favor of https://gerrit.wikimedia.org/r/510994. I still think this is a better solution, but the other one is good enough that I'm not interested in spending more time on this.

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

In T160690#5251867, @matmarex wrote:

The first user is probably going to be this MobileFrontend patch: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/510067/10..11/extension.json

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/516573 :P