Optimize and tidy up in mediawiki.page.ready/ready.js
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Od1n
	Feb 5 2023, 3:19 PM

Description

Optimize some jQuery selectors. Similar to T324523, which you may read for discussions and benchmarks. Much faster on browsers that lack optimizations, and a good practice.
- Not done, see discussion on Gerrit
An use of classList.contains() (refs Element.classList and DOMTokenList.contains()). Implemented in all supported browsers and cleaner. Also faster… very marginally.
- Done

Details

	Subject	Repo	Branch	Lines +/-
	Optimize and tidy up in mediawiki.page.ready/ready.js	mediawiki/core	master	+1 -1

Customize query in gerrit

Related Objects

Mentioned In: T349935: TypeError: Cannot read properties of undefined (reading 'contains')
Mentioned Here: T324523: Optimize some jQuery selectors

Event Timeline

Od1n created this task.Feb 5 2023, 3:19 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 5 2023, 3:19 PM

Change 886473 had a related patch set uploaded (by Gerrit Patch Uploader; author: Anne Haunime):

[mediawiki/core@master] Optimize and tidy up in mediawiki.page.ready/ready.js

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

gerritbot added a project: Patch-For-Review.Feb 5 2023, 3:21 PM

Pushed a commit to also optimize some selectors that use the [data-mw="interface"] attribute (refs https://github.com/wikimedia/mediawiki/commit/b1e3f9e38d7c74e080e99e58dac55629b9f0b097).

Note I replaced table.diff with .diff otherwise the optimization wouldn't take place. Precisely thanks to the [data-mw="interface"] attribute, the selector stays robust.

Izno added projects: Performance-Team, MediaWiki-User-Interface.Feb 5 2023, 6:04 PM

larissagaulia moved this task from Inbox, needs triage to Radar on the Performance-Team board.Feb 6 2023, 7:53 PM

larissagaulia edited projects, added Performance-Team (Radar); removed Performance-Team.

Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.Feb 14 2023, 6:31 PM

Krinkle edited projects, added Wikimedia-Performance-recommendation; removed Performance-Team (Radar).Aug 18 2023, 8:41 PM

There's been a lengthy discussion about the change in Gerrit. The consensus seems to be that the classList is an obvious improvement, but the jQuery selectors changes are less obvious: they are faster in some benchmarks, but not necessarily always in practice, this code is not in a hot loop anyway so it doesn't really matter, and the change make the code slightly more complicated to read.

I'd like to amend the change to undo the jQuery selectors changes and keep the classList change, and then merge it. @Od1n I assume you're the original author (although the Gerrit Patch Uploader attribution confuses me a bit), please let me know if you'd be okay with that (or please amend the change yourself if you'd like).