1465588 - Consider removing dashboard code

Status: RESOLVED FIXED

(Tree Management :: Perfherder, enhancement, P1)

Description

[William Lachance (:wlach)]

:rwood added some generic dashboard code back in the day which was very useful for e10s, hasal, and something other things. However, it seems like this hasn't been used much lately. To save the hassle of converting it to react with the rest of perfherder/treeherder, I would suggest removing it for now. It will still be in version history if we want to use it as inspiration later.

:rwood -- do you have any objections to doing this? If not this might make a good bug to work on before the rest of the perfherder react conversion.

Comment 1

[Robert Wood [:rwood]]

It depends if we foresee the need for a similar dashboard. At one point I believe we were planning to have a similar one for talos vs talos with webrender (?). If that's not needed anymore, and :jmaher doesn't see the need for a similar comparison type of dashboard in the future then I'm all for removing the code.

Comment 2

[Ed Morley [:emorley]]

I don't think that's really the question though?

If a dashboard *is* needed in the future, then "remove framework remnants now, add a full React dashboard later" is basically the same amount of work as "convert framework remnants to React now, populate new dashboard implementation later". (And arguably less work, since it will allow seeing the wood for the trees.)

However if a dashboard *isn't* needed in the future, then "remove framework remnants for now" saves a lot of wasted effort.

As such, removing now seems preferable - unless there is a factor we've overlooked? :-)

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

we will probably have a new dashboard in July to compare firefox vs chrome for benchmarks in perfherder...so yeah, we will be using this relatively soon.

Comment 4

[William Lachance (:wlach)]

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #3)
> we will probably have a new dashboard in July to compare firefox vs chrome
> for benchmarks in perfherder...so yeah, we will be using this relatively
> soon.

Depending on the requirements, maybe it makes more sense to create this new dashboard outside of perfherder (just calling perfherder api endpoints), perhaps as a seperate (maybe even react-based) site?

Anyway, we can leave the code for now to keep our options open.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

I am fine with an external dashboard- in fact that makes a lot of sense to keep perfherder simpler.  I think we had aspirations of making a generic compare view between user defined parameters- that seems unrealistic.

From my point of view knowing nothing about react, working with an existing template as prior art is a quick way to solve the problem.  Making something standalone requires an endpoint to hit, but either way- if there is prior art I can be the one editing/creating dashboards without needing to spend a month (realistic time to get 40 hours of focus time) learning react.

Comment 6

[William Lachance (:wlach)]

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #5)
> I am fine with an external dashboard- in fact that makes a lot of sense to
> keep perfherder simpler.  I think we had aspirations of making a generic
> compare view between user defined parameters- that seems unrealistic.
> 
> From my point of view knowing nothing about react, working with an existing
> template as prior art is a quick way to solve the problem.  Making something
> standalone requires an endpoint to hit, but either way- if there is prior
> art I can be the one editing/creating dashboards without needing to spend a
> month (realistic time to get 40 hours of focus time) learning react.

Makes sense, I think iodide (https://github.com/iodide-project/iodide) might be a promising solution for small projects like these, being quickly able to develop small interactive dashboards with custom behaviour is one of its main use cases. We could probably port a lot of the existing compare logic to a notebook without much trouble. Ping me when the time comes and we can discuss. :)

Comment 7

[Armen [:armenzg]]

Let me know if there's anything for me.

For the curious, here's a prototype (read clunky) that hits the various Perfherder APIs to plot various benchmarks:
https://perf-goggles.netlify.com/linux64/ARES6

Comment 8

[Sarah Clements [:sclements]]

Can someone please update me on what specifically should not be converted for the dashboard, if anything?

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Thanks for checking in here :sclements.  As we are moving to a trend to use Armen's new graphing libraries and making perfherder be a data source/sheriffing tool, then I would like to say remove the comparison dashboard code.

The code would be related to these files:
https://github.com/mozilla/treeherder/commit/4bee8f1702ca20349ad3dc52743bce155fb0acb8#diff-086c5ae474e8466cb5a2c8b5d9694bdc

we used to have these easily accessible from perfherder, but haven't used them in a long time.

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4211

Comment 11

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/debac0c5c2e2c4623a1582b57a2afdbf9340d242
Bug 1465588 - Remove unused Perfherder dashboard code (#4211)

Comment 12

[Ed Morley [:emorley]]

Could you open a follow-up PR to address the remaining review comments?
https://github.com/mozilla/treeherder/pull/4211#discussion_r229649135
https://github.com/mozilla/treeherder/pull/4211#discussion_r229649624

Comment 13

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4222

Comment 14

[Sarah Clements [:sclements]]

(above pull request #4222 addresses missed review comments)

Comment 15

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/e590908b69c3d5ffef3b36fca7de15b366588617
Bug 1465588 - Remove dead perf dashboard code part2 (#4222)

remove baseTitle and newTitle values from compare.js and into comparetable.html partial. Remove releaseBlockerCriteria, unused 'blocker' refs and getTrendMap method from compare.js. Delete comparetable partial and trendtable partials .