1266116 - Sheriff panel -Job Exclusion editor gives: a web page is slowing down your browser

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P1)

Description

[Cameron Dawson [:camd]]

Editing or creating a new Job Exclusion takes forever to load the page and usually gives the "show javascript" message of "a web page is slowing down your browser"

We have so many job types that this panel is taking forever to load just because of the ng-repeat for that list.

We should investigate better ways to edit these exclusions.  ReactJS may be a good option to explore here.  I've seen a simple test that can render thousands of items in a list fairly quickly.

Comment 1

[William Lachance (:wlach)]

We should perhaps also consider purging out-of-date jobs from the database. I think there are more than a few that are no longer around.

Comment 2

[William Lachance (:wlach)]

(out of date job types and groups, that is)

Comment 3

[William Lachance (:wlach)]

(In reply to William Lachance (:wlach) from comment #1)
> We should perhaps also consider purging out-of-date jobs from the database.
> I think there are more than a few that are no longer around.
> (out of date job types and groups, that is)

Filed/fixed bug 1268593 for this.

Comment 4

[William Lachance (:wlach)]

At some point we probably want to rethink the whole idea of job types. Really we have probably just a few hundred of these at most (e.g. "Mochitest"), but since there is no other way to differentiate chunk number or test type we're overloading these to store that. If we encoded that information as part of the job itself, we would have a much more reasonable # of items to display here.

Comment 5

[William Lachance (:wlach)]

I believe the performance has gotten worse here mainly due to the issue I described in comment 4. There are now 9000+ job types in treeherder. Much space to be taken up with three types of jobs which seem to be arbitrarily adding version and/or locale specific metadata to their job type: "funsize" (2795 rows, e.g. "[funsize] MAR signing task (today-4, chunk en-US)"), "beetmover" (1014 rows, e.g. "[beetmover] firefox mozilla-beta win32 locales partials candidates 7/10"), "Firefox UI" (2634 rows, e.g. "Firefox UI Update Tests - beta es-AR-49.0b1")

I still maintain this metadata doesn't belong in the job type table, and that we should store it somewhere else (maybe as part of the job itself). It's not only a performance issue but also a UI one -- a sheriff shouldn't have to select 2000+ firefox ui job types to exclude them.

Comment 6

[Jonathan Griffin (:jgriffin)]

Cam, addressing this is a sheriff ask. Can you take a look soon?

Comment 7

[Cameron Dawson [:camd]]

Absolutely.  I'm trying to wrap up my quarterly deliverables (at least the first one which is the primary one) which are super close.  But I was going to hit this right afterward.  

I'm planning on using ReactJS to render instead of Angular ng-repeat. (I have a branch with this partially done, fwiw)

I agree with Will that we should re-think how we store this data.  Though this change is still absolutely valuable, even after the re-think.

Comment 8

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:exclusion-editor > mozilla:master

Comment 9

[Cameron Dawson [:camd]]

Comment on attachment 8786457 [details] [review]
[treeherder] mozilla:exclusion-editor > mozilla:master

I'm not sure if Ed or Will would rather (be willing to) review this.  I only really need one of you to do it, Sorry it's kind of big.  Lots of the main logic is copied from the Sheriff panel and controller, though I did my best to clean it up where I could, so you should really treat it as all new code, tbh.

Thanks guys!

Comment 10

[William Lachance (:wlach)]

I think having multiple reviewers on a big change like this is good. I'm happy to look at the patch from an angular-best-practices point of view on Monday.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8786457 [details] [review]
[treeherder] mozilla:exclusion-editor > mozilla:master

I think this looks great! The design looks very sensible.

Comment 12

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/de9868d613f4b02371cecf8b8b4c102583ddfe9f
Bug 1266116 - Use wildcard in grunt usemin instead of listing file

https://github.com/mozilla/treeherder/commit/fb435816b06ba4a77a897a82a049de3747cb2cc8
Bug 1266116 - Add the Admin page: Full-page exclusion editor

This moves the Sheriff panel out to a full-page app.  The functionality
is basically the same, however.  Much of the logic and HTML were just
copied from the Sheriff panel.

This also introduces ReactJS to the repo.  This was used to speed up
the rendering of the reference data lists in the Exclusions detail
editor.  The ``reactselect`` component is defined as normal JavaScript.
We decided not to introduce using JSX at this time.