Page MenuHomePhabricator

Performance review of GlobalWatchlist
Closed, ResolvedPublic

Description

Description

MediaWiki-extensions-GlobalWatchlist is being developed for deployment on WMF production. The extension creates a "global" watchlist display at Special:GlobalWatchlist, configurable at Special:GlobalWatchlistSettings and displayed via javascript. The watchlist is retrieved via the Watchlist API and is "global" in the sense that it can interact with multiple sites, but also works with a just a single site

Preview environment

At Special:GlobalWatchlist on the beta cluster's metawiki. Requires having pages on your watchlist with changes to see.

Which code to review

Extension code on gerrit - patchset TBD

Performance assessment

Please initiate the performance assessment by answering the below:

  • What work has been done to ensure the best possible performance of the feature? There is a "fast mode" option for users that should result in better performance by reducing the number of API queries made (only fetch a list of pages with updates, not all changes, and don't fetch tag information for sites or try to display Wikibase labels)
  • What are likely to be the weak areas (e.g. bottlenecks) of the code in terms of performance? Everything for getting the changes to display is done via the standard api, which might slow things down compared to generating the display in php (as done at Special:Watchlist)
  • Are there potential optimisations that haven't been performed yet? One of the grant requirements is to limit the number of sites that a user can try to show at once (T258935: Add a limit to the number of sites a user can watch)
  • Please list which performance measurements are in place for the feature and/or what you've measured ad-hoc so far. If you are unsure what to measure, ask the Performance Team for advice: performance-team@wikimedia.org.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Gilles triaged this task as Medium priority.

When I first heard of this project, I tried out the ancestor of it you had made as a gadget. Having seen it in action, my main concern was not just limits but also defaults. It seems like the assumptions about the default wikis one would be interested in were a bit excessive.

What's the plan for determining the best set of defaults for a given user? IMHO the most cautious approach in terms of performance would be to only start with the wiki this feature is first invoked on have the user opt into more wikis manually.

When I first heard of this project, I tried out the ancestor of it you had made as a gadget. Having seen it in action, my main concern was not just limits but also defaults. It seems like the assumptions about the default wikis one would be interested in were a bit excessive.

What's the plan for determining the best set of defaults for a given user? IMHO the most cautious approach in terms of performance would be to only start with the wiki this feature is first invoked on have the user opt into more wikis manually.

Per the grant requirements at https://meta.wikimedia.org/wiki/Grants_talk:Project/DannyS712/Create_a_global_watchlist_extension#Round_1_2020_decision

The extension should only be available on Meta.
Wikis used should be all opt-in (eg. by default you would only see your Meta watchlist). The current userscript has 4 default wikis and that's too many.

This was implemented, and by default only the local wiki is included.

That all sounds good. Ping me here once the extension is deployed on Beta, so that I can try it out in a real deployment context.

I recommend that you track performance from the user's perspective. For instance, the time it takes between requesting the special page to them being able to see all the elements from their global watchlist. This can be done fairly simply with mw.track. The data will then go to EventLogging and be easily displayed in a dashboard. Looking at the high percentiles, you should be able to see if there are some serious outliers worth investigating. You should track as many timings this way that you think are relevant, for instance you might want to track "fast mode" separately.

That all sounds good. Ping me here once the extension is deployed on Beta, so that I can try it out in a real deployment context.

Will do

I recommend that you track performance from the user's perspective. For instance, the time it takes between requesting the special page to them being able to see all the elements from their global watchlist. This can be done fairly simply with mw.track. The data will then go to EventLogging and be easily displayed in a dashboard. Looking at the high percentiles, you should be able to see if there are some serious outliers worth investigating. You should track as many timings this way that you think are relevant, for instance you might want to track "fast mode" separately.

Will do, but that will be based on the number of changes that need to be processed, eg whether there needs to be a continuation in the api calls or if just one is enough. It'll be pretty hard to get that tested well on the beta cluster

These metrics will probably only be actionable once the extension is deployed in production. The idea is to catch performance issues we might not have foreseen on Beta. I don't expect that they will reveal much on Beta, where the content and the performance profile have nothing to do with production.

Gilles changed the task status from Open to Stalled.Sep 11 2020, 7:45 AM

Marking this as stalled for my own tracking of performance reviews that are currently actionable for us. Please move it back to "open" once the extension is running on Beta and we can test it there. Thanks!

@Gilles 985256a359b46723f27e22b346dbdd4d314bf6a4 added a new resourceloader module, ext.guidedTour.globalWatchlistSettings, which is conditionally registered based on whether the guided tour extension is available.
In T264817: Improve GuidedTour performance, @Krinkle wrote

Its code base not kept up with current best practices. I consider it a prerequisite for any new use/deployment of the extension, for it to first have an active steward, be updated to current best practices, and specifically to have a significantly better startup performance footprint for regular page views.

Does this mean that, in its current form, the GlobalWatchlist extension cannot be deployed, and the GuidedTour needs to be removed from the extension in order to move forward?

DannyS712 changed the task status from Stalled to Open.Nov 18 2020, 10:59 PM

Marking this as stalled for my own tracking of performance reviews that are currently actionable for us. Please move it back to "open" once the extension is running on Beta and we can test it there. Thanks!

Now installed on beta

Not a performance concern, but I notice that the settings page is a navigation dead end. Once you get to it, there is no link back to the Special:GlobalWatchlist page, you're forced to use user history. And once you save changes, there's no link to either Special:GlobalWatchlist or Special:GlobalWatchlistSettings in its default state.

Refreshing the page at that stage doesn't work, you get stuck with seeing this:

I think that the experience of the settings page should match Special:Preferences. After saving your changes, you should still see the form. And there needs to be a button to go back to Special:GlobalWatchlist.

Having to manually type the URL of wikis to watch is really cumbersome. There should be a drop-down list to pick from. Furthermore, there's nothing stopping me from entering and saving a website that doesn't exist:

Which results in a request actually sent to any website I entered in there:

There is a window error, but the requests still happened. There needs to be a whitelist here, someone could get tricked into putting a non-Wikimedia site in those settings.

Plus, despite the error, the site in question ends up being listed on the page anyway:

I see a lot of debug messages in the console. Those should be restricted to MediaWiki's debug mode (adding debug=true to the page).

Also, inviting the user via a window alert to consult errors in the console isn't the normal error display pattern on MediaWiki. Most people don't know what the browser console is. The error should be displayed on the page itself for users who aren't technical to be able to find it easily. It also needs to be localised, the console error messages aren't.

The "mark as seen" feature doesn't seem to work, at least in the live update mode. What I've just marked as seen just comes back on the next live update.

Stopping the live update doesn't seem to take effect immediately. The next scheduled update and its requests still happen. I imagine that it's a settimeout underneath which doesn't check the state of the live update button when it runs or something along those lines.

Now, in terms of performance. The live update mode generates a lot of requests. Those requests keep happening even when the browser tab is in the background. This must be changed to requests only happening when the current page/tab is in focus. There's no point running those updates for a tab that the user doesn't see.

I also notice that there's a CORS preflight request for every request to wikis other than meta. I don't know if the default can be changed, but it would be nice if the responses of those OPTIONS requests were cached for some time. At the moment they're explicitly no-cache. If you get the CORS green light for GET/POST through an OPTIONS requests for that particular API URL, there's no reason that you wouldn't get it again a few seconds later... Even being cached for a few minutes would basically cut the amount of requests generated by the live updates by half.

When you manually hit the "refresh" button, there's a progress bar that appears for a split second. Same when you load Special:GlobalWatchlist initially. The best practice in terms of performance perception is to avoid displaying progress bars for processes that are short most of the time. You should wait until a few seconds of waiting period have passed before displaying a progress bar. This way the majority of users who have a fast internet connection won't see the progress bar at all. Progress bars draw attention to the wait, and makes it feel longer when it's actually short.

Not a performance concern, but I notice that the settings page is a navigation dead end [snip]

Filed T268259: Add a way to get back to Special:GlobalWatchlist from Special:GlobalWatchlistSettings

Having to manually type the URL of wikis to watch is really cumbersome. There should be a drop-down list to pick from.

Filed T268261: Add a list or autocompletion for sites on Special:GlobalWatchlistSettings

Furthermore, there's nothing stopping me from entering and saving a website that doesn't exist: [snip]

Already planning to address at T268210: User-defined project domains should be validated

I see a lot of debug messages in the console. Those should be restricted to MediaWiki's debug mode (adding debug=true to the page).

They are only shown when $wgGlobalWatchlistDevMode is true, which is true for the beta cluster but false for prod (or will be). Is that enough?

Also, inviting the user via a window alert to consult errors in the console isn't the normal error display pattern on MediaWiki. Most people don't know what the browser console is. The error should be displayed on the page itself for users who aren't technical to be able to find it easily. It also needs to be localised, the console error messages aren't.

Filed T268264: Better front end error reporting

The "mark as seen" feature doesn't seem to work, at least in the live update mode. What I've just marked as seen just comes back on the next live update.

The changes are being marked as seen, but the filter to only show unseen changes isn't enabled. Filed T268263: Add `unread` flag to api query calls

Stopping the live update doesn't seem to take effect immediately. [snip]

Filed T268265: Stopping the live update doesn't seem to take effect immediately


Now, in terms of performance. The live update mode generates a lot of requests. Those requests keep happening even when the browser tab is in the background. [snip]

Filed T268266: Only run live updates when tab is being viewed

I also notice that there's a CORS preflight request for every request to wikis other than meta. [snip]

Filed T268267: Reduce CORS preflight requests

When you manually hit the "refresh" button, there's a progress bar that appears for a split second. [snip]

Filed T268268: Don't show loading bar immediately


Thanks for starting the review. I should be able to address most of the issues raised soon.

They are only shown when $wgGlobalWatchlistDevMode is true, which is true for the beta cluster but false for prod (or will be). Is that enough?

Sounds acceptable, but why have a dedicated global for this, instead of using the existing MediaWiki debug mode?

They are only shown when $wgGlobalWatchlistDevMode is true, which is true for the beta cluster but false for prod (or will be). Is that enough?

Sounds acceptable, but why have a dedicated global for this, instead of using the existing MediaWiki debug mode?

I wanted to be able to debug logs more easily on the beta cluster and when testing without resorting to using the general debug mode which might affect other things

@Gilles most of the tasks I filed for the notes you raised have been resolved, except for T268266: Only run live updates when tab is being viewed which has a patch pending review and T268261: Add a list or autocompletion for sites on Special:GlobalWatchlistSettings which will take some more thinking about regarding implementations. Would you be willing to take another look?

@Gilles most of the tasks I filed for the notes you raised have been resolved, except for T268266: Only run live updates when tab is being viewed which has a patch pending review and T268261: Add a list or autocompletion for sites on Special:GlobalWatchlistSettings which will take some more thinking about regarding implementations. Would you be willing to take another look?

{{ping}}

I don't have anything new to request, besides the tasks and patches already in flight.