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.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Inline Parser::firstCallInit() into ::__construct() | mediawiki/core | master | +17 -27 | |
| Deprecate Parser::firstCallInit() | mediawiki/core | master | +45 -3 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T261329 Prepare Parsoid for MW | |||
| Open | None | T236809 Refactor Parser.php to allow alternate parser (Parsoid) | |||
| Open | None | T236812 Parser.php should be split into a base class and a parser implementation | |||
| Open | cscott | T236811 Parser creation should always use factory | |||
| Open | None | T250444 Deprecate and remove Parser::firstCallInit() |
Event Timeline
Comment Actions
Change 589437 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] WIP: Deprecate Parser::firstCallInit()
Comment Actions
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.
Comment Actions
Change 589437 merged by jenkins-bot:
[mediawiki/core@master] Deprecate Parser::firstCallInit()
Comment Actions
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().
Comment Actions
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:
- https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Poem/+/wmf/1.35.0-wmf.30/extension.json#19
- https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Poem/+/wmf/1.35.0-wmf.30/includes/Poem.php#16
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?
Comment Actions
Change 665155 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Inline Parser::firstCallInit() into ::__construct()
Comment Actions
Change 665155 merged by jenkins-bot:
[mediawiki/core@master] Inline Parser::firstCallInit() into ::__construct()