650382 - Update endurance results dashboard to use pre-calculated values for min/max/average

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Result Dashboard, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Use the new pre-calculated values stored in the report rather than working this out on the client-side.

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use pre-calculated values from report. v1

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 526405 [details] [diff] [review]
Use pre-calculated values from report. v1

>-              minAllocatedMemory : min(testAllocatedMemoryResults),
>-              maxAllocatedMemory : max(testAllocatedMemoryResults),
>-              averageAllocatedMemory : Math.round(average(testAllocatedMemoryResults)),
>-              minMappedMemory : min(testMappedMemoryResults),
>-              maxMappedMemory : max(testMappedMemoryResults),
>-              averageMappedMemory : Math.round(average(testMappedMemoryResults))

Please also remove the declaration for those array variables which hold the data. Otherwise looks fine.

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use pre-calculated values from report. v1.1

(In reply to comment #2)
> Comment on attachment 526405 [details] [diff] [review] [review]
> Use pre-calculated values from report. v1
> 
> >-              minAllocatedMemory : min(testAllocatedMemoryResults),
> >-              maxAllocatedMemory : max(testAllocatedMemoryResults),
> >-              averageAllocatedMemory : Math.round(average(testAllocatedMemoryResults)),
> >-              minMappedMemory : min(testMappedMemoryResults),
> >-              maxMappedMemory : max(testMappedMemoryResults),
> >-              averageMappedMemory : Math.round(average(testMappedMemoryResults))
> 
> Please also remove the declaration for those array variables which hold the
> data. Otherwise looks fine.

Thanks for spotting that, it should be a bit cleaner now.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 529956 [details] [diff] [review]
Use pre-calculated values from report. v1.1

I think it's fine now. Lets wait for the other patches to get checked-in.

Comment 5

[u415141]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/13565931

Comment 6

[u415141]

Henrik Skupin changed story state to started in Pivotal Tracker

Comment 7

[u415141]

Henrik Skupin changed story state to unstarted in Pivotal Tracker

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use pre-calculated values from report. v2.0

Updated to support reports that don't include the pre-calculated stats. Metrics will be output as "--" on this older reports.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 535672 [details] [diff] [review]
Use pre-calculated values from report. v2.0

>+          value.min_allocated_memory = (value.stats && value.stats.allocated) ? Math.round(value.stats.allocated.min * BYTE_TO_MEGABYTE) : METRIC_UNAVAILABLE;

For old reports which do not pre-calculate the metrics there will be no stats property. Why can't we simply check it once at the beginning? That way we wouldn't have to run the same check again and again. Why do you have an extra check for stats.allocated?

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I'm checking for stats.allocated because results from Firefox 6+ will have the explicit and resident values, and not allocated or mapped.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I was hoping that you cover that in the patch on bug 657508, so we do not mix those different features. It would also make problems if we would have to back-out this specific patch.

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use pre-calculated values from report. v2.1

(In reply to comment #11)
> I was hoping that you cover that in the patch on bug 657508, so we do not
> mix those different features. It would also make problems if we would have
> to back-out this specific patch.

Updated patch with suggestions.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 536309 [details] [diff] [review]
Use pre-calculated values from report. v2.1

Looks good. Go ahead and land it.

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Landed as:
https://github.com/whimboo/mozmill-dashboard/commit/bac9de248fad62d96a648a5a3dbf71e8f74c98ed