Closed Bug 413714 Opened 16 years ago Closed 16 years ago

Cleanup talos displays on Tinderbox

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rcampbell, Assigned: anodelman)

References

Details

Attachments

(1 file)

Based on our discussions in the Perfmeeting and IRC, we should clean up the talos displays in Tinderbox. Proposed format is:

T{test}(1):{time}({unit})

Including a link in {test} to the historical data, and a link on the 1 to just the current run.

Current tests in the form of Tp_l can be renamed to just Tp.

E.g., T<a href="http://graphs.mozilla.org/dgraph.html#name=tp_loadtime&spst=range&spstart=0&spend=392&bpst=cursor&bpstart=0&bpend=392&m1tid=112452&m1bl=0&m1avg=0">p</a>(<a href="http://graphs.mozilla.org/graph.html#spst=range&spstart=0&spend=1201112490&bpst=cursor&bpstart=0&bpend=1201112490&m1tid=61188&m1bl=0&m1avg=0">1</a>:695.56
I guess you'll have to imagine how those links will look. The example was supposed to show:

Tp(1):695.56
UX Nit - the tinderbox waterfall is big enough, and info-dense enough, that I tend to view it zoomed out, and even by default it has small fonts.  Can we use {time} as a link target to the most recent run, just to increase click surface?  The '1' is kinda wee.
What does the "(1)" indicate?
(In reply to comment #2)
> UX Nit - the tinderbox waterfall is big enough, and info-dense enough, that I
> tend to view it zoomed out, and even by default it has small fonts.  Can we use
> {time} as a link target to the most recent run, just to increase click surface?
>  The '1' is kinda wee.

and get rid of the (1) altogether? So, for Tp, Tp would link to the historical data and {time} to the single run.

That makes sense to me. Any objections?
(In reply to comment #4)
> 
> That makes sense to me. Any objections?

Sounds good
By {time} you mean the actual time number?  That seems like it would work, though it may be confusing... at least until people get used to it, especially since it'll be two links that are adjacent to each other.  But sure, let's do it.
(In reply to comment #6)
> By {time} you mean the actual time number?  That seems like it would work,
> though it may be confusing... at least until people get used to it, especially
> since it'll be two links that are adjacent to each other.  But sure, let's do
> it.

Do you think it would be worthwhile to add link titles, so that there is hoverhelp?  

"Historical Data for Tp on this machine" and
"Details for this testing run"

Maybe?  Am I overcomplicating your once-simple change?
So is {time} just the word "time" with a link to the single run results, or do you actually want a datestamp in there?  Or the buildid?

Just making sure that I really understand what people want before I start playing with this...
(In reply to comment #8)
> So is {time} just the word "time" with a link to the single run results, or do
> you actually want a datestamp in there?  Or the buildid?
> 
> Just making sure that I really understand what people want before I start
> playing with this...

Ah, no, we're using Rob's notation from comment 0 and 1, so aiui the text will read:

  Tp: 695.56

With |Tp| linked to historical data for Tp and |695.56| linked to individual run details. 
Does anyone really look at the details of individual runs?  I would think that it is pretty ingrained to click on the number for a run and get that run graphed against previous results.  But, I guess that that habit would change pretty quickly, especially since linking to individual test run results on the graph server currently caused the browser to freeze...
yes, people look at individual runs when trying to see where a potential regression is occurring.

Johnath's interpretation of my non-EBNF link grammar is correct. {time} ::= digit+(.digit+)
(In reply to comment #10)
> Does anyone really look at the details of individual runs?  I would think that
> it is pretty ingrained to click on the number for a run and get that run
> graphed against previous results.  But, I guess that that habit would change
> pretty quickly, especially since linking to individual test run results on the
> graph server currently caused the browser to freeze...
> 

Yes I know I do - but it is a lower priority than the topline numbers - so we can move the links to the bottom of the output (or a separate page)
Couple of quick thoughts:

1) We can drop the %CPU time and the memset numbers (private bytes are more useful)
2) I think the detailed 'per run' data is the less likely used and I worry that folks are used to clicking on the number to get the historials not the details:

So how about:

<a href="graph">Tp:248ms</a> <a href="dgraph">(details)</a>

(In reply to comment #13)
> Couple of quick thoughts:
> 
> 1) We can drop the %CPU time and the memset numbers (private bytes are more
> useful)
> 2) I think the detailed 'per run' data is the less likely used and I worry that
> folks are used to clicking on the number to get the historials not the details:
> 
> So how about:
> 
> <a href="graph">Tp:248ms</a> <a href="dgraph">(details)</a>
> 

Oh and I test per line.   
Bad, IMO; "(details)" takes up horizontal room, which is at a premium:  How about:

<a graph>Ts:123</a>
<a graph>Tp:248</a>
<a graph>Tdhtml:152</a>
<br>
Details:<br>
<a dgraph>Ts</a>|<a draph>Tp</a>|<a dgraph>Tdhtml</a>

?

We have tons of vertical space in there, we can use it a lot more.

(Without the HTML:)

Ts:123
Tp:248
Tdhtml:152

Details
Ts | Tp | Tdhtml

Are we still interested in collecting the Private Bytes and %Cpu?  It is easy enough to turn that off entirely and just have Memset/rss.
Don't think it hurts, though next week we're going to have to sit down and figure out what to do about the db size; we might not want a database at all, but something more optimized for our data.
(In reply to comment #15)

> We have tons of vertical space in there, we can use it a lot more.
> 
> (Without the HTML:)
> 
> Ts:123
> Tp:248
> Tdhtml:152
> 
> Details
> Ts | Tp | Tdhtml
> 

Fine with me.  Think we have enough cooks in this kitchen :-)
Remove %cpu and private bytes from the output (though we are still collecting them and sending them to the graph server).  Output like:

tsvg: 3790.27
twinopen: 134.25
tp: 54.00
tp_memset: 28151808.00

Details:
| tsvg | twinopen | tp | tp_memset |

This looks good when I display the html chunk in a browser, the question will be whether or not it looks good in the actual waterfall.
Assignee: nobody → anodelman
Status: NEW → ASSIGNED
Attachment #299325 - Flags: review?(rcampbell)
Comment on attachment 299325 [details] [diff] [review]
new talos output for tinderbox print statements

lines 52-53, 60-61:
+      url = url_format % (results_server, values[0],)
+      link = link_format % (url, linkName,)

should remove the trailing commas in the tuples. Looks good otherwise.
Attachment #299325 - Flags: review?(rcampbell) → review+
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.22; previous revision: 1.21
done

Now to sit back and wait for a talos cycle...
The initial push on this is complete.  We still want to do some fine tuning but that can be put into other bugs.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Mass move of Core:Testing bugs to mozilla.org:Release Engineering:Talos. Filter on RelEngTalosMassMove to ignore.
Component: Testing → Release Engineering: Talos
Product: Core → mozilla.org
QA Contact: testing → release
Version: Trunk → other
Component: Release Engineering: Talos → Release Engineering
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: