Page MenuHomePhabricator

Deprecate and remove Parser::firstCallInit()
Open, Needs TriagePublic

Description

Parser::firstCallInit() should just be done in the constructor (or, eventually, superclass constructor). We don't need lazy initialization of the Parser object anymore since the MediaWikiServices infrastructure handles lazy creation for us.

Event Timeline

cscott created this task.Apr 16 2020, 10:13 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 16 2020, 10:13 PM

Change 589437 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] WIP: Deprecate Parser::firstCallInit()

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

This patch is complete, but I think it could use review from the Performance-Team before being merged. From my ad hoc testing it appears that the parser is indeed lazily created and so this patch doesn't add performance cost, but I'd appreciate another pair of eyes and/or more tools brought to bear to ensure there's not some corner case where we're accidentally creating an unused parser object and slowing things down.

I did notice a few places that were creating Parser objects as a result of localizing messages through MessageCache::getParser(). That code path already always calls Parser::firstCallInit() so this patch wouldn't slow things down any -- however it's possible there are speedups possible if we can arrange that MessageCache::parse() isn't invoked where it's possible to avoid it.

Change 589437 merged by jenkins-bot:
[mediawiki/core@master] Deprecate Parser::firstCallInit()

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

The point of firstCallInit() is that it's not called from setHook(), so extensions calling $wgParser->setHook() from an extension function do not trigger firstCallInit(). CodeSearch shows that there are at least five extensions that are still doing that, none of them are used by WMF though. For example YetAnotherKeywords. We've had lazy-loading of Parser via StubObject for a long time -- the lack of lazy loading was not the reason for firstCallInit().

cscott added a subscriber: demon.Apr 30 2020, 7:24 AM

I think that usage has already been affected by the deprecation and removal of the global $wgParser, though (T160811). The reliable way to set a tag hook in modern MediaWiki is via the ParserFirstCallInit hook (which isn't going away), the way Extension:Poem does it, for example:

That ensures that your hook is present in any created Parser objects, not just the single "global" $wgParser. (As described in 2009 by @demon in 8b3bdbd96f930f74d811834955cde404a9b8cc38).

What am I missing?