1166994 - Loading review requests with lots of sub-requests and/or lots of diffs is slow

Status: RESOLVED INVALID

(MozReview Graveyard :: General, defect, P3)

Description

[Mike Hommey [:glandium]]

Take https://reviewboard.mozilla.org/r/7951/ for example. It has 11 sub-requests. It takes, here, 4s to load.

Now, take a diff, like https://reviewboard.mozilla.org/r/7951/diff/13/ : it does 72 requests, mostly sequentially, with no cache (urls even contain a timestamp), and overall, takes close to 30s here because of all the round trips and the fact that I'm in Japan, light-speed is not infinite, and my ping time to rb.mozilla.org looks like this:

40 packets transmitted, 40 received, 0% packet loss, time 39049ms
rtt min/avg/max/mdev = 141.567/239.583/341.105/76.884 ms

Taking the average, 72 sequential round trips alone account for 17s.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Same diff as the linked one, for comparison purpose

Comment 2

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #1)
> Created attachment 8608423 [details] [diff] [review]
> Same diff as the linked one, for comparison purpose

As a data point, that attachment is a copy of https://reviewboard.mozilla.org/r/7951/diff/13/raw/. Loading it in splinter takes 1.69s here. I get that splinter does less things, but you can understand how going from less than 2s to 30s is not really appealing.

Comment 3

[Mark Côté [:mcote]]

Part of this will be fixed in bug 1135396.

Comment 4

[:glob ✱]

> Now, take a diff, like https://reviewboard.mozilla.org/r/7951/diff/13/ : it
> does 72 requests, mostly sequentially

bug 1251286 is tracking the non-parallelism of the requests.

> with no cache (urls even contain a timestamp)

review board uses the If-None-Match header to provide an etag to the server, which returns 304 if the html render of the diff is unchanged.

> and overall, takes close to 30s here because of all the round
> trips and the fact that I'm in Japan, light-speed is not infinite, and my
> ping time to rb.mozilla.org looks like this:
> 
> 40 packets transmitted, 40 received, 0% packet loss, time 39049ms
> rtt min/avg/max/mdev = 141.567/239.583/341.105/76.884 ms

meanwhile, in perth australia... :)

40 packets transmitted, 40 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 218.461/227.104/242.846/11.265 ms

> Taking the average, 72 sequential round trips alone account for 17s.

i think the sequential requests issue is the crux of this report, duplicating to bug 1251286.

Comment 5

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.