Page MenuHomePhabricator

ResourceLoader's mw.templates Map is full of empty objects
Closed, ResolvedPublic

Description

Visit any page on enwiki or mw.org and in the browser console inspect mw.templates , e.g.

mw.templates.get()
mw.templates.get('user.tokens')
var foo = 0; for (val in mw.templates.get()) { foo++}

there are 100+ keys in there, one for each loaded module. They're all empty objects, bar mediaWiki.templates.values["mediawiki.action.view.postEdit"] (which actually does have a template).

It's not breaking anything, but it's unexpected. I think the fix is to check if module's templates $.isEmptyObject() before adding, patch coming.

Event Timeline

Spage created this task.Feb 7 2015, 1:48 AM
Spage raised the priority of this task from to Low.
Spage updated the task description. (Show Details)
Spage added subscribers: Spage, Jdlrobson.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 7 2015, 1:48 AM

<del>(gerritbot didn't update the task as I would expect)</deL>
grr, can't have newline between Bug: and Change-ID

Change 189158 had a related patch set uploaded (by Legoktm):
Don't add empty objects to mw.templates

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

Patch-For-Review

Change 189616 had a related patch set uploaded (by Krinkle):
resourceloader: Omit empty parameters from mw.loader.implement calls

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

Patch-For-Review

Change 189158 abandoned by Krinkle:
Don't add empty objects to mw.templates

Reason:
Actually, I did that already in December last year, but it had to be reverted to an unforeseen side-effect. This has been resolved. See I9e998261ee9b.

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

Change 189616 merged by jenkins-bot:
resourceloader: Omit empty parameters from mw.loader.implement calls

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

I think all these patches are merged and deployed on beta labs, but by the time mediawiki.js's execute() is called, there is an empty registry[module].templates object for dozens of module names, so mw.templates still has 70 objects in it, nearly all empty.

Krinkle closed this task as Resolved.Jul 8 2015, 2:35 PM
Krinkle claimed this task.
Krinkle added a subscriber: Krinkle.

I think all these patches are merged and deployed on beta labs, but by the time mediawiki.js's execute() is called, there is an empty registry[module].templates object for dozens of module names, so mw.templates still has 70 objects in it, nearly all empty.

Yeah, but they're not sent over the wire. They're created at run time because those fields are of type Object and they must have a value, so they default to empty object.

This isn't specific to templates. Though templates is the only one with a public getter. Inspect mw.loader.moduleRegistry to see many empty arrays for dependencies, for example.

I'm closing this since the redundant parameters are no longer sent in load.php responses. The memory footprint of these objects existing can be evaluated in a separate issue. I believe in most cases we can easily remove these since this is a private structure. However dependencies is a field we look through very frequently and enables native batching as an array. The others (including templates) can probably be removed.

However due to having a public getter it is also the hardest one to remove because right now callers can assume that passing a loaded module's name results in an object that they can try accessing keys on. So we may want to keep that for back-compat in the getter only.