load.php slow to respond with styles, even with cached LESS parser output
Closed, ResolvedPublicBUG REPORT

Description

Cache invalidation in FileModule::compileLessString() is broken on my local setup (Git master), resulting in LESS recompilation whenever styles are requested, even when none of the files have changed. I debugged this and found that it has to do with skins being located outside the MediaWiki installation directory.

Specifically, my local setup symlinks the extensions and skins into MediaWiki's extensions and skins directories. For each imported file, Less_Parser calls realpath(), which resolves symlinks. RelPath then sees the file is located outside the MediaWiki installation directory and adds ../ at the beginning. So when there is a mix of core and skin file imports, and FileContentsHasher sorts the paths, the order (and thus the computed hash) depends on whether RelPath was used (../skins/ < resources/ < skins/). The problem is, of course, that while the relative paths are used for checking validity of cached output, the absolute paths are used when computing the hash when saving to the cache. It seems that this bug was introduced along with the use of RelPath in 5e0fa6ecc811debb.

Steps to replicate the issue:

  • Enable the APCu PHP extension for the relevant SAPI
  • Check out MediaWiki core and the Vector skin in two separate directories, such as DIR/core and DIR/skins/Vector
  • Create a symlink: ln -s ../../skins/Vector DIR/core/skins/
  • Install MediaWiki
  • Open the Main Page, or alternatively, directly request load.php?lang=en&modules=skins.vector.styles.legacy&only=styles&skin=vector
  • Force-reload the page a few times

What happens?: Responses for this specific load.php URL take a long time (2 seconds), with nearly all of that time spent within Less_Parser.

What should have happened instead?: Second and subsequent responses should have been quick, as LESS compiler output had already been cached.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change #1282363 had a related patch set uploaded (by PleaseStand; author: PleaseStand):

[mediawiki/core@master] Consistently use relative paths when hashing LESS imports

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

Change #1284761 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] ResourceLoader: Simplify FileContentsHasher test

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

Change #1284762 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] ResourceLoader: Fix stable sort in FileContentsHasher

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

Change #1284761 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Simplify FileContentsHasher test

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

Krinkle triaged this task as Medium priority.
Krinkle moved this task from Inbox to Confirmed Problem on the MediaWiki-ResourceLoader board.

Change #1284762 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Fix stable sort in FileContentsHasher

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

@PleaseStand Can you confirm that the issue is resolved in latest master with the above patch?

Presuming solved for now. Feel free to re-open or file a new bug anytime if you find otherwise.

Change #1282363 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Expand cached Less paths before hashing

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