Page MenuHomePhabricator

Overall performance review of Popups/Hovercards extension
Closed, ResolvedPublic


Overall performance of the extension before we make it the default behavior on Chinese, Greek and Catalan wikipedia. It would be nice to get T67845 reviewed too.

Event Timeline

Prtksxna raised the priority of this task from to High.
Prtksxna updated the task description. (Show Details)
Prtksxna added a project: Page-Previews.
Prtksxna added subscribers: Aklapper, Prtksxna.
Aklapper renamed this task from Overall performance review to Overall performance review of Popups/Hovercards extension.Feb 2 2015, 6:32 PM
Aklapper set Security to None.
Aklapper added a project: Performance Issue.
greg claimed this task.
greg added subscribers: aaron, ori, greg.

@ori and @aaron have decided not to do pre-deploy performance reviews for now.

I definitely want to review this one, though.

Last time I checked (quite a while ago), Hovercards were using uncacheable API calls. I pointed that out at the time but it wasn't a priority for the beta release.

I don't think it would be a problem on the server side load-wise but it would hurt a lot the end-user performance due to RTT, especially for this particular country selection.

@ori, all blockers are resolved. This is ready for review now.

Sorry for the delay in completing the performance review. The fault is entirely my own.

It looks OK for initial deployment. I don't think we should transition from opt-in to all page-views on a major wiki like zhwiki without intermediate steps, though, so I recommend restricting the initial deployment to Greek and Catalan wikis.

There does appear to be a blocker, though: is unmerged.

I just re-checked on Greek Wikipedia. Hovercard AJAX responses come with "Cache-control: private, must-revalidate, max-age=0" (from MediaWiki) which means they are not edge-cached. I mentioned this before above — @ori, any reason you didn't comment on it?

In my opinion, this is not ready to be deployed more widely and I would argue that we're not doing it any justice by having it deployed to languages whose primary audiences are far away from origin/eqiad.

Jdlrobson subscribed.

Given @ori comment about being the only blocker which is now merged.