Page MenuHomePhabricator

[Perf trend] "skins.minerva.content.styles" is the slowest module to build
Closed, ResolvedPublic

Description

Per https://grafana.wikimedia.org/#/dashboard/db/resourceloader.

It takes about 0.25s to generate the styles for this module alone. The HTTP request that includes this module also builds about a dozen other modules, so this impacts performance. Incidentally, while not the most-frequently built, it is the #2 most-frequently built. (Over 50 times per minute globally.).

I imagine most of this is taken up by Less compilation.

Event Timeline

Krinkle raised the priority of this task from to Needs Triage.
Krinkle updated the task description. (Show Details)
Krinkle set Security to None.
Krinkle added subscribers: Krinkle, Aklapper.

Interesting. Looking at the module it consistents of 12 less files so I suspect this may be a case of file reads (I don't see anything unusual in the less file itself)

Note the module is loaded on every page on mobile skin.

@Krinkle are there any existing tools to debug LESS compilation to work out the bottle neck?
Could this be masking a bigger problem with our choice of LESS library that might warrant more eyes on https://www.mediawiki.org/wiki/Requests_for_comment/Change_LESS_compilation_library?

Apps team you use this module right? How do you use it? It's possible apps could be contributing to cache misses in some way.

This is the only place the module gets added http://git.wikimedia.org/blob/mediawiki%2Fextensions%2FMobileFrontend.git/2e992fd1c8e4a22c57f3913fb67f6bce42d6f582/includes%2Fskins%2FSkinMinerva.php#L1127

I would thus expect builds for this module to be the same as base.styles
Is any other extension making use of this?

KLans_WMF subscribed.

Assigning to @bmansurov fro some more investigation

The module in question:

	'skins.minerva.content.styles' => $wgMFResourceFileModuleBoilerplate + array(
		'position' => 'top',
		'styles' => array(
			'resources/skins.minerva.content.styles/main.less',
			'resources/skins.minerva.content.styles/thumbnails.less',
			'resources/skins.minerva.content.styles/images.less',
			'resources/skins.minerva.content.styles/galleries.less',
			'resources/skins.minerva.content.styles/headings.less',
			'resources/skins.minerva.content.styles/blockquotes.less',
			'resources/skins.minerva.content.styles/lists.less',
			'resources/skins.minerva.content.styles/links.less',
			'resources/skins.minerva.content.styles/text.less',
			'resources/skins.minerva.content.styles/tables.less',
			'resources/skins.minerva.content.styles/hacks.less',
		),
	),

On my local machine these are the numbers (in seconds):

The module is ready in 0.578547000885.

Compile time (in lessphp) of each less file:
0.0511178970337
0.0528738498688
0.0531129837036
0.0592210292816
0.0508549213409
0.0484960079193
0.0475759506226
0.0460000038147
0.0539209842682
0.0437369346619
0.0543348789215
Total compile time: 0.5612454414

Notice above that no single file stands out.

As we can see 97% (0.5612454414 / 0.578547000885) of the module load time is taken up by the compile time.

So I moved over the file contents from all other less files in the module to the main.less and ran the above test.

The module is ready in 0.0968840122223
main.less is compiled in 0.0827250480652

Edit: @Krinkle suggested that I import less files into a single file and run the tests again. Here are the results.

The module is ready in 0.125006914139
index.less is compiled in 0.105803012848

The contents of index.less is:

@import "main.less";
@import 'main.less';
@import 'thumbnails.less';
@import 'images.less';
@import 'galleries.less';
@import 'headings.less';
@import 'blockquotes.less';
@import 'lists.less';
@import 'links.less';
@import 'text.less';
@import 'tables.less';
@import 'hacks.less';

Conclusion: reading multiple files is expensive.

Recommendation: combine multiple less files into one and separate content using comments (as opposed to using different file names).

Interesting. Thanks for exploring @bmansurov!
I guess we should take this opportunity to have a standard entry point for all ResourceLoader modules where there are more than one stylesheet.
e.g. styles.less

Do you want to submit a patch to fix this and we'll make another card to do this for others?

I also wonder if there is a way for ResourceLoaderFileModule to wrap all the list of files in one single compiled less file.. this way we will fix performance for all modules built with less.

(Note: it's worth pointing out that this will make less files harder to debug but obviously performance trumps that)

Change 227594 had a related patch set uploaded (by Bmansurov):
Reduce the module build time

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

I also wonder if there is a way for ResourceLoaderFileModule to wrap all the list of files in one single compiled less file.. this way we will fix performance for all modules built with less.

(Note: it's worth pointing out that this will make less files harder to debug but obviously performance trumps that)

Currently each file is checked for existence and an error is thrown if the file doesn't exist. I think it's a good idea to investigate this option.

Change 227594 merged by jenkins-bot:
Reduce the module build time

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

Assigning to myself to check after the deployments of this week if the build time shows better on grafana.

For the purposes of the sprint this is done, but don't close it yet.