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

RESOLVED FIXED

Status

Tree Management Graveyard
TBPL
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: armenzg, Assigned: sfink)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
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
(Assignee)

Comment 1

6 years ago
Created attachment 608024 [details] [diff] [review]
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.)
Attachment #608024 - Flags: review?(arpad.borsos)
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
Attachment #608024 - Flags: review?(arpad.borsos) → review+
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.
(Assignee)

Comment 4

6 years ago
(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

6 years ago
SQUIRREL!
(Assignee)

Comment 6

6 years ago
Created attachment 608070 [details] [diff] [review]
SQUIRREL!

(In reply to Jeff Walden (:Waldo) (remove +bmo to email) from comment #5)
> SQUIRREL!
Attachment #608070 - Flags: review?(jwalden+bmo)

Comment 7

6 years ago
Comment on attachment 608070 [details] [diff] [review]
SQUIRREL!

arf+
Attachment #608070 - Flags: review?(jwalden+bmo) → review+
Depends on: 738891
Duplicate of this bug: 573671
This was fixed in March :-)
Assignee: nobody → sphink
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.