In T396248#11018725, @Bewfip wrote:I suspect https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/1148378 is causing MediaWiki\Extension\DiscussionTools\Hooks\PageHooks::onOutputPageBeforeHTML to run on every normal article page. From the flame graph archive at https://performance.wikimedia.org/php-profiling/, this function is barely visible on 2025-06-13, but begins to call BatchModifyElements::apply and RemexHtml after 2025-06-14. The time wasted by this is considerable on long pages.
Description
Description
Details
Details
Related Changes in Gerrit:
Customize query in gerrit
Related Objects
Related Objects
Event Timeline
Comment Actions
This seemed interesting, and a much worse effect on performance than I expected from that change (T396248#10892247), so I decided to have a look.
I found a really cool visualization at MediaWiki Entrypoint Profiling dashboard, and it says that time spent in DiscussionTools code increased, from being below the threshold, to ~8%, suddenly on 2025-06-20. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/1148378 was deployed in the week of 2025-06-10, which was the week before, so this should be something different.
 |
Comparing individual flame graphs zoomed in, on 2025-06-06 DiscussionTools's onOutputPageBeforeHTML handler did not take enough time to appear on the graph, on 2025-06-16 and 2025-06-18 it takes a modest amount of time (this seems expected for change 1148378), then on 2025-06-20 it's hugely increased.
 |  |  |  |
At this point I am pretty sure that 1148378 can't be the cause, but something has definitely happened on or around 2025-06-20. (By the way, on 2025-06-11 DiscussionTools was deployed to a number of wikis by default (https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1155295), but that date doesn't match the time at all.)
I looked at the Server Admin Log for 2025-06-19 and 20, and I didn't find anything interesting, other than "group2 to 1.45.0-wmf.6" deployment. So I looked at the DiscussionTools changes in wmf.6, and I think I found the cause: change 1159441: Remove placeholders when features disabled. That was a follow-up to the change you pointed out, and it would indeed cause that code to run on every page view; we should have thought of that.
It's also interesting that this impact doesn't seem obvious on the Backend Pageview Timing dashboard. There are some noisy changes in the graph that don't look like an 8% increase to me. Maybe it would look like 8% if you applied enough statistics math to it.
Comment Actions
Change #1171312 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Only parse and process the HTML if it seems to contain our tags
Comment Actions
Change #1171312 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Only parse and process the HTML if it seems to contain our tags
Comment Actions
I have an idea for follow-up: Can the OutputPageBeforeHTML hook return early if the ParserAfterTidy hook does not find any comment during Parser::parse()? It would be helpful for those content pages in $wgExtraSignatureNamespaces, which can be long sometimes. This does not concern the talk namespaces, and I guess the "empty state" UI elements might depend on it.
(Occasionally I have the urge to mass-add __NOTALK__, although I know that's impractical :P)
Comment Actions
I had a look and I don't think that would be easy to do. DiscussionTools adds some markup (<mw:dt-ellipsisbutton>) for every heading on the page in the ParserAfterTidy hook, even if it has no comments, and so the OutputPageBeforeHTML hook needs to postprocess that markup. Maybe this could be refactored somehow, as currently in the majority of cases the latter hook ends up just removing that markup, but this seems a bit complex and easy to get wrong (especially in case we end up changing the configuration in the future so that the markup is used more often). The current approach where we return early only if there is no DiscussionTools markup at all is safer, and seems good enough to me.
I have noticed though that we still look for DiscussionTools subscribe buttons to postprocess even if we know that the page has no DiscussionTools markup. That seems like an easier improvement.
Comment Actions
Change #1190389 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Only parse and process the HTML if it seems to contain our tags (for topic subscriptions)
Comment Actions
Change #1190389 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Only parse and process the HTML if it seems to contain our tags (for topic subscriptions)