731261 - Loading a commit change with a lot of changes is super-slow

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Armen [:armenzg]]

For instance, if you load this change you will feel what I mean:
https://tbpl.mozilla.org/?tree=Oak&rev=eca94af42dda

This normally happens when a merge happens towards a branch that is out of sync from mozilla-central for few days.

Can we please trim it after showing 20 changes?

If they want the whole thing they can load the pushlog:
https://hg.mozilla.org/projects/oak/pushloghtml?changeset=eca94af42dda

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Show at most 20 changesets from a push

Would it be enough to just suppress the display, or do we need to throw them away earlier? This patch suppresses them from being displayed (though the "list changeset urls" will still show the full list.)

Comment 2

[Arpad Borsos [:Swatinem]]

Comment on attachment 608024 [details] [diff] [review]
Show at most 20 changesets from a push

Review of attachment 608024 [details] [diff] [review]:
-----------------------------------------------------------------

Hm I´ve never heard the word elide/elision before. So I learn something new every day :)

::: js/UserInterface.js
@@ +1084,5 @@
> +    var preElision = Math.floor(max / 2);
> +    var elide = (patches.length > max) ? patches.length - max : 0;
> +    if (max == 0)
> +      elide = 0;
> +    console.log("elide = " + elide + "/" + patches.length);

please remove that console.log

Comment 3

[Markus Stange [:mstange]]

By the way, there's a very old patch in bug 573671 which aims to add a clickable expander, but the patch is probably hopelessly out of date.

Comment 4

[Steve Fink [:sfink] [:s:]]

(In reply to Arpad Borsos (Swatinem) from comment #2)
> Comment on attachment 608024 [details] [diff] [review]
> Show at most 20 changesets from a push
> 
> Review of attachment 608024 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hm I´ve never heard the word elide/elision before. So I learn something new
> every day :)

Probably unnecessarily jargon-y, I guess.

> 
> ::: js/UserInterface.js
> > +    console.log("elide = " + elide + "/" + patches.length);
> 
> please remove that console.log

Doh! Oops.

(In reply to Markus Stange from comment #3)
> By the way, there's a very old patch in bug 573671 which aims to add a
> clickable expander, but the patch is probably hopelessly out of date.

Oh, sure enough. Although a goal of this patch was to improve the loading time, so I'd kinda want to ajaxify the expand button. Not that this is going to help a huge amount, given that it's still loading and parsing all of the changesets.

But I just ran off the end of my attention span, so I'll stick with this for now.

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/460bd555d870

Comment 5

[Jeff Walden [:Waldo]]

SQUIRREL!

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: SQUIRREL!

(In reply to Jeff Walden (:Waldo) (remove +bmo to email) from comment #5)
> SQUIRREL!

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 608070 [details] [diff] [review]
SQUIRREL!

arf+

Comment 9

[Ed Morley [:emorley]]

Comment on attachment 608024 [details] [diff] [review]
Show at most 20 changesets from a push

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/460bd555d870

Comment 10

[Ed Morley [:emorley]]

This was fixed in March :-)