As our team is becoming familiar with all of the available code libraries to implement client side features, we're finding learning opportunities, and T317117 is one of those. In this task, we're going to optimize a feature based on our evolving understanding of how to contain the jquery emanating from OOUI. We will improve performance and legibility in the process.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| mobile preferences: trim insertToggles() comments | mediawiki/core | master | +5 -17 | |
| mobile preferences: optimize toggle switches | mediawiki/core | master | +29 -33 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • Samwalton9-WMF | T308651 Make Special:Preferences available to mobile web users | |||
| Resolved | • Samwalton9-WMF | T311708 Implement MVP for new mobile Preferences interface | |||
| Resolved | jsn.sherman | T323231 Improve code quality for Mobile Preferences toggle switches |
Event Timeline
Comment Actions
Change 857747 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):
[mediawiki/core@master] mobile preferences: optimize toggle switches
Comment Actions
Test wiki created on Patch demo by JSherman (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/96f76bc491/w
Comment Actions
Test wiki created on Patch demo by JSherman (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/313dbcd50d/w
Comment Actions
I've created a gerrit change request with several patchset that take individual optimization steps. I've noted the local runtime that I logged for the function and updated it in the comments. The first patch set just names the function to make it easier to profile (for a baseline), then the following patch sets progressively optimize the code and update the measured runtime for the function in the comment.
I'm creating patchdemos for the baseline and final patch sets so I we can all check it out live on our machines. There will be run-to-run variability, and I did multiple runs locally when I was testing this, but I'm including some one off checks from the patchdemos here.
To do some basic performance profiling:
baseline (~62ms):
- login to the baseline patchdemo https://patchdemo.wmflabs.org/wikis/96f76bc491
- go to preferences and select the mobile view
- in chrome/edge/chromium open dev tools and select the performance tab
- click the "start profiling and reload page" icon near the top left of the performance panel
- when profiling is complete, click into the the summary and hit ctrl + f
- enter insertToggles into the search box
- note the aggregated time
optimised (~13ms):
- login to the optimized patchdemo https://patchdemo.wmflabs.org/wikis/313dbcd50d
- go to preferences and select the mobile view
- in chrome/edge/chromium open dev tools and select the performance tab
- click the "start profiling and reload page" icon near the top left of the performance panel
- when profiling is complete, click into the the summary and hit ctrl + f
- enter insertToggles into the search box
- note the aggregated time
Comment Actions
Change 857747 merged by jenkins-bot:
[mediawiki/core@master] mobile preferences: optimize toggle switches
Comment Actions
Sam Walton noticed a bug for T317117 in beta which patch 857747 inadvertently resolves, so I merged it for expediency. We still intend to spend more time looking at our code quality here and will also need to remove the educational comments. Patches related to that will be filed against this task.
Comment Actions
Change 863017 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):
[mediawiki/core@master] mobile preferences: trim insertToggles() comments
Comment Actions
Change 863017 merged by jenkins-bot:
[mediawiki/core@master] mobile preferences: trim insertToggles() comments
Comment Actions
Test wiki on Patch demo by JSherman (WMF) using patch(es) linked to this task was deleted:
Comment Actions
Test wiki on Patch demo by JSherman (WMF) using patch(es) linked to this task was deleted: