Page MenuHomePhabricator

[Regression] MediaViewer hook adds 30% to Backend Pageview Timing on mobile (300 -> 400ms)
Closed, ResolvedPublic5 Estimated Story Points

Description

https://grafana.wikimedia.org/d/QLtC93rMz/backend-pageview-timing

Screenshot 2026-09-24 35.png (2,455×617 px, 85 KB)

https://grafana.wikimedia.org/d/rLMucuf4k/mediawiki-entrypoint-profiling?from=now-90d&to=now&timezone=utc&var-entrypoint=index_view&var-http_method=GET&viewPanel=panel-2

Screenshot 2026-09-24 53.png (2,479×1,327 px, 465 KB)

.. this started Mon 21 Sep in the 14:00 UTC hour:

Screenshot 2026-09-24 36.png (1,425×380 px, 125 KB)

.. which lines up with this Server Admin Log entry:

2026-09-21 13:22 <mfossati@deploy1003> Finished scap sync-world: Backport for [[gerrit:1343122|Let AA measure eligible readers w/o beta opt-in (T437076)]] (duration: 14m 19s)

https://performance.wikimedia.org/arclamp/svgs/daily/2026-09-18.excimer-wall.index.svgz?s=MultimediaV (no samples)
https://performance.wikimedia.org/arclamp/svgs/daily/2026-09-23.excimer-wall.index.svgz?s=MultimediaV (5% of samples)
https://performance.wikimedia.org/arclamp/svgs/daily/2026-09-23.excimer-wall.index.svgz?s=MultimediaV&x=816.9&y=1730

Screenshot 2026-09-24 16.png (2,382×1,575 px, 536 KB)

It does quite a lot of work, most of which does not belong in the critical path of a pageview, such as parsing non-trivial page content via the Parser or Parsoid. This parsing is non-trivial because it seems to involvke database queries, as well an HTTP call with Guzzle/Curl to the Commons API where it then does even more wikitext parsing, namely parsing the file description page, to extract attribution metadata.

I don't know what stage of rollout this feature is in or whether it was config-enabled or train-enabled, but on Tue-Wed on group1, it already added +100ms/30% to mobile pageview latency.

Why this matters especially now

Because we're actively trying to increase capacity for logged-in users. An SLO is in the works which would enforce this.

Details

Related Changes in Gerrit:
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

This probably also contributed to the issues we saw during services (and mw-ro) switchover to codfw T438896: Investigate high saturation in codfw

Change #1345096 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Remove reliance on file repo

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

Change #1345109 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Remove reliance on ParserOutput

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

Above patches should fix things, I believe. I've slapped them together hastily, so please review thoroughly!

Change #1345109 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove reliance on ParserOutput

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

Change #1345096 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Remove reliance on file repo

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

matthiasmullie raised the priority of this task from High to Unbreak Now!.Sun, Sep 27, 7:23 PM
matthiasmullie lowered the priority of this task from Unbreak Now! to High.

Change #1345369 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Remove reliance on ParserOutput

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

Change #1345370 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Remove reliance on file repo

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

Change #1345369 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Remove reliance on ParserOutput

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

Change #1345370 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Remove reliance on file repo

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

Mentioned in SAL (#wikimedia-operations) [2026-09-28T07:08:05Z] <mlitn@deploy1003> Started scap sync-world: Backport for [[gerrit:1345070|[MultimediaViewer] Testwiki use same carousel config as other wikis]], [[gerrit:1345371|Fix jump to image break in carousel retest (T439252)]], [[gerrit:1345369|Remove reliance on ParserOutput (T439182)]], [[gerrit:1345370|Remove reliance on file repo (T439182)]]

Mentioned in SAL (#wikimedia-operations) [2026-09-28T07:26:45Z] <mlitn@deploy1003> mlitn: Backport for [[gerrit:1345070|[MultimediaViewer] Testwiki use same carousel config as other wikis]], [[gerrit:1345371|Fix jump to image break in carousel retest (T439252)]], [[gerrit:1345369|Remove reliance on ParserOutput (T439182)]], [[gerrit:1345370|Remove reliance on file repo (T439182)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verifi

Mentioned in SAL (#wikimedia-operations) [2026-09-28T07:40:12Z] <mlitn@deploy1003> Finished scap sync-world: Backport for [[gerrit:1345070|[MultimediaViewer] Testwiki use same carousel config as other wikis]], [[gerrit:1345371|Fix jump to image break in carousel retest (T439252)]], [[gerrit:1345369|Remove reliance on ParserOutput (T439182)]], [[gerrit:1345370|Remove reliance on file repo (T439182)]] (duration: 32m 06s)

Change #1345502 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Skip expensive work if known to be useless

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

Change #1345512 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Cache extracted carousel data

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

The 2 backported patches should get rid of these crossed-out sections; about 2/3 of the overall cost in this hook (and external calls)

Screenshot_2026-09-24_16.png (2,382×1,575 px, 623 KB)

The circled sections, however, remain, and we can't get rid of them without significantly affecting the feature.
While we could potentially off-load bits to the frontend, the DOM parsing is needed to weed out certain files (e.g. used for decorative rather than content purposes), and not doing this server-side would mean a significant layout shift

I submitted 2 more patches that should at least tamp down on the size of these:

  • One to skip the expensive code path altogether if cached parser output already indicates there are less than 3 images
  • Another to cache the results of whatever remains for a day

Change #1345502 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Skip expensive work if known to be useless

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

Krinkle renamed this task from [Regression] MediaViewer hook adds 30% to Backend Pageview Tining on mobile (300 -> 400ms) to [Regression] MediaViewer hook adds 30% to Backend Pageview Timing on mobile (300 -> 400ms).Mon, Sep 28, 2:24 PM

Change #1345512 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Cache extracted carousel data

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

Change #1345626 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Skip expensive work if known to be useless

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

Change #1345632 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Cache extracted carousel data

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

Change #1345626 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Skip expensive work if known to be useless

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

Change #1345632 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.21] Cache extracted carousel data

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

Mentioned in SAL (#wikimedia-operations) [2026-09-29T07:04:49Z] <mlitn@deploy1003> Started scap sync-world: Backport for [[gerrit:1345626|Skip expensive work if known to be useless (T439182)]], [[gerrit:1345632|Cache extracted carousel data (T439182)]], [[gerrit:1345627|Make carousel images are visible in the margins (T439422)]], [[gerrit:1345633|carousel: fix carousel image preload observer root (T439422)]]

Mentioned in SAL (#wikimedia-operations) [2026-09-29T07:09:06Z] <mlitn@deploy1003> mlitn: Backport for [[gerrit:1345626|Skip expensive work if known to be useless (T439182)]], [[gerrit:1345632|Cache extracted carousel data (T439182)]], [[gerrit:1345627|Make carousel images are visible in the margins (T439422)]], [[gerrit:1345633|carousel: fix carousel image preload observer root (T439422)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now

Mentioned in SAL (#wikimedia-operations) [2026-09-29T07:18:28Z] <mlitn@deploy1003> Finished scap sync-world: Backport for [[gerrit:1345626|Skip expensive work if known to be useless (T439182)]], [[gerrit:1345632|Cache extracted carousel data (T439182)]], [[gerrit:1345627|Make carousel images are visible in the margins (T439422)]], [[gerrit:1345633|carousel: fix carousel image preload observer root (T439422)]] (duration: 13m 39s)

I think we've taken this about as far as we can for now. Calls to external services are gone; the remaining more intensive processing is avoided as much as possible, and cached where not.

The overall "Backend Pageview Timing (quantile estimate)", while it's been going down, hasn't quite come down to 2-weeks-ago numbers yet.
But I'm not seeing indications that MultimediaViewer is a significant contributor to latency anymore.

Week ago (wall time 2.58%): https://performance.wikimedia.org/arclamp/svgs/hourly/2026-09-22_09.excimer-wall.all.svgz?s=MultimediaV&x=920.2&y=2034

Screenshot 2026-09-29 at 12.27.36.png (1,199×610 px, 208 KB)

Yesterday (wall time 1.30%), first 2 fixes: https://performance.wikimedia.org/arclamp/svgs/hourly/2026-09-28_09.excimer-wall.all.svgz?s=MultimediaV&x=926.2&y=1730

Screenshot 2026-09-29 at 13.36.32.png (1,202×434 px, 141 KB)

Now (wall time 0.48%), all fixes: https://performance.wikimedia.org/arclamp/svgs/hourly/2026-09-29_09.excimer-wall.all.svgz?s=MultimediaV&x=920.4&y=1810

Screenshot 2026-09-29 at 12.28.40.png (1,200×369 px, 117 KB)


@Krinkle Any objections against us moving forward with this experiment at this point? "Moving forward", AIUI, meaning: the current experiment (started on 21/09) will end, and we will run another experiment under the same conditions (at least on the backend - frontend will render things differently)

@matthiasmullie We're still up +20% from ~280ms to ~340ms for Backend Pageview Timing p75 mobile, and likewise the underlying >250ms bucket.

See also Excimer profile for a mobile pageview and One component: ext_MultimediaViewer.

Screenshot 2026-09-29 at 14.08.22.png (2,462×1,329 px, 392 KB)

The heart of the problem is parsing the article HTML into a DOM. This is not a cost you can control because it grows linearly with the size of the article with no way to limit it, e.g. even if you only extract 1 image, you'll have parsed the whole article. That is too much to do ad-hoc in an extension. We design for the cache miss. To give some context: It is this "second parsing" cost that delayed the Parsoid-readviews rollout.

Unless this pattern was documented or recommended by the Content Transform Team, I would have to object to moving forward.

Does this feature really require sitting in a layer above and separate from the rest of MediaWiki?

The image carrousel cache doesn't seem to vary by skin or user language, though, so it seems like this work could be done behind the main ParserCache, and keep only a small array in $extApi->getMetadata()->setExtensionData. Example: Cite, Example: Kartographer, Parsoid docs.

For some extensions, the answer to that question is Yes, and they really need a "second" parse (e.g. due to their logic varying by skin or user language, thus unsuitable for main ParserCache). MediaWiki core has a transform pipeline for this purpose (along along with a "post-processing" cache), which gives extensions a centralised "second" parse where they all share the result of a a single parse and single cache, without multiplying costs. This one "second" parse pipeline has still been tough on latency, but many people have spent months making that work. We can't afford multiple of these. If a second parse is a requirement for you, I suggest using this pipeline. It hands you an already-parsed DOM. You could store a small result array in ParserOutput::setExtensionData, to retrieve cheaply on pageviews. Example: onParserOutputPostCacheTransform

Minor notes for incremental impovements that may buy you some time, in case moving to post-processing transform can't work:

  • Looking at the above Excimer profile and Logstash capture, I see there is also an added database query on each page as well: MediaWiki\Page\PageProps::getProperties [1.2ms] [0 rows] db1218: SELECT pp_page,pp_propname,pp_value FROM page_props WHERE pp_page = 534366 AND pp_propname = 'nomediaviewercarousel'. You can read these for free from the ParserOutput object in OutputPage. Ref T347123.
  • In addition to a second parsing of the article HTML, there also appears to be a second roundtrip to memcached to download the article HTML, and a second JSON parse of that large blob. This is coming from the getCachedParserOutput call. If you reuse the ParserOutput object that's already in OutputPage, you should be able to avoid that. Ref Codesearch: onOutputPageParserOutput for examples, either acting on it direclty (like CheckUser), or grab and stash in OutputPage::setProperty for the next hook (like Disambiguator). The latter accounts for cases where onOutputPageParserOutput may be called multiple times. I've filed T439585 to make this easier to do in the future.

Change #1346570 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Lose DB call for page prop

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

Change #1346571 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Process DOM in hook instead of rebuilding

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

Does this feature really require sitting in a layer above and separate from the rest of MediaWiki?

Post-processing could definitely work here, if not for the caveat that it's an experiment for now - not a full rollout.

Please correct me if I'm wrong, but a random pageview would not invoke wtPostprocess, so that data would not be available immediately - not until a new edit/purge, or until the output falls out of cache - is that correct?
I imagine there may be a way to invoke it, but I would expect that to be even more expensive than the current DOM parse.
If indeed post-processing results would not immediately be available, that is a problem while this is an experiment: it would only run for 30 days, and require accurate usage data from the start rather than being dependent on whenever pages are parsed next.

If/once that is no longer the case post-experiment, I don't currently see any other reason why we couldn't then move to the post-processing route (although it's not entirely clear to me ATM whether that's a Parsoid-only thing, or also functional in the legacy parser process, but that's something to look into at some later point)

For some extensions, the answer to that question is Yes, and they really need a "second" parse

Done in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MultimediaViewer/+/1346571

there is also an added database query on each page as well

Done in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MultimediaViewer/+/1346570

@matthiasmullie My assumption, given the size of the regression, is that this experiment is at a high enough roll-out that computing the data unconditonally behind ParserCache would be a net-improvement (You can continue to read/display the data conditonally from the same Skin/OutputPage hook as today.) However, perhaps this assumption is wrong.

What size percentage experiment is this? That is, ignoring any control group or non-experiment group doing baseline work, what percentage needs to do special work? Is the carousell data computed on <=1% of mobile pageviews or closer to 100% of mobile pageviews? (I mean purely the experiment percentage upperbound, so without static conditions about namespace or page content).

You're right that when data is computed behind the cache, it is not instantly live. One way to address that is to enable the computation 1-2 days ahead of the user-visible experiment (possibly using a very low non-zero value to trigger your feature flag, or make it a separate flag). The majority of pageviews are on pages with a ParserCache expiry of <=24h, so 1-2 days should suffice to have your A-B test take effect everywhere at once.

This delay seems unavoidable. The expensive nature of the work seems it can't happen on-demand, and thus can't start everywhere at once without warmup. I guess the exception would be if you varied the ParserCache key (or dynamically detect and reject data that lacks your array property via onRejectParserCacheValue), but it would rather risky to suddenly invalidate ParserCache everywhere at once. However, in a sense that's exactly what the previous approach did by adding Parser demand in front of the ParserCache (with no cache, or an initially empty cache).

Change #1346698 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Extract images in parser post-processing

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

1-2 days should suffice to have your A-B test take effect everywhere at once

I was expecting significantly longer caches - that's great news!
I just moved everything into Parsoid post-processing: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MultimediaViewer/+/1346698 - another quick look would be much appreciated, @Krinkle :)

I do believe this pipeline is Parsoid-only, though, right? We do have 1 wiki (zhwiki) in this experiment where I believe Parsoid has not yet rolled out - would defaulting to the existing implementation there be acceptable for the time being if there's a good reason we can't leave it out?

Change #1347686 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.22] Lose DB call for page prop

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

Change #1347687 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.22] Extract images in parser post-processing

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

Change #1346570 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Lose DB call for page prop

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

Change #1346698 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Extract images in parser post-processing

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

Change #1347686 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.22] Lose DB call for page prop

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

Change #1347687 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@wmf/1.47.0-wmf.22] Extract images in parser post-processing

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

Mentioned in SAL (#wikimedia-operations) [2026-10-01T20:37:50Z] <mlitn@deploy1003> Started scap sync-world: Backport for [[gerrit:1347686|Lose DB call for page prop (T439182)]], [[gerrit:1347687|Extract images in parser post-processing (T439182)]], [[gerrit:1347729|Make carousel open MMV directly (T439611)]], [[gerrit:1347735|Removes image preview from Treatment 1-2 (T439611)]]

Mentioned in SAL (#wikimedia-operations) [2026-10-01T20:41:51Z] <mlitn@deploy1003> mlitn, ksarabia: Backport for [[gerrit:1347686|Lose DB call for page prop (T439182)]], [[gerrit:1347687|Extract images in parser post-processing (T439182)]], [[gerrit:1347729|Make carousel open MMV directly (T439611)]], [[gerrit:1347735|Removes image preview from Treatment 1-2 (T439611)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2026-10-01T20:48:46Z] <mlitn@deploy1003> Finished scap sync-world: Backport for [[gerrit:1347686|Lose DB call for page prop (T439182)]], [[gerrit:1347687|Extract images in parser post-processing (T439182)]], [[gerrit:1347729|Make carousel open MMV directly (T439611)]], [[gerrit:1347735|Removes image preview from Treatment 1-2 (T439611)]] (duration: 10m 56s)

Change #1348436 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/extensions/MultimediaViewer@master] Don't run image extraction unless enabled

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

Change #1346571 abandoned by Matthias Mullie:

[mediawiki/extensions/MultimediaViewer@master] Process DOM in hook instead of rebuilding

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

@Krinkle Above work (moving the processing to post-processing, behind ParserCache) has been merged & deployed yesterday evening.
I don't think there's anything left to do at this moment, so please let us know if you should sport more issues, or comment/close if you feel this has sufficiently been resolved.
FYI we plan to move forward with the experiment next week - let us know if you have concerns.

@matthiasmullie This looks great!

You can see the p75 latency drop back down to 300ms after your deploy. And likewise, the target bucket (<=250ms) grows back to 70% of requests. Nice!

Screenshot 2026-10-02 at 17.14.35.png (1,589×651 px, 111 KB) Screenshot 2026-10-02 at 17.14.15.png (1,339×802 px, 73 KB) Screenshot 2026-10-02 at 17.26.05.png (2,724×826 px, 200 KB)

From Grafana: Backend Pageview Timing and Entrypoint Profiling: One component

Your deploy was around 20:30 UTC, and around UTC midnight we have an hour or so of concentrated daily ParserCache expiries which (expectedly) temporarily raise it up, which is part which is part of the daily cadence. Here is the same data zoomed out to encompass the previous baseline.

Screenshot 2026-10-02 at 17.16.01.png (2,719×1,411 px, 372 KB) Screenshot 2026-10-02 at 17.16.23.png (2,719×1,438 px, 223 KB)