The fix for bug 656869 requires some changes in order for the dashboard to display the results.

Initial patch submitted for feedback.

Comment on attachment 532785 [details] [diff] [review] Updated dashboard to show results from Firefox 6.0. v1 Review of attachment 532785 [details] [diff] [review]: ----------------------------------------------------------------- Code-wise this patch looks fine to me -- was there some more specific feedback you were looking for?

Comment on attachment 532785 [details] [diff] [review] Updated dashboard to show results from Firefox 6.0. v1 Thanks. I just wanted to gain some confidence before submitting for review. If you didn't see anything obviously bad then that's great.

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

At the moment the charts display all possible metrics (with the values of 0 for any not present). I would be interested to see if you have any ideas for suppressing this data - I couldn't see a way to do it in the template... A sample report can be seen here: http://mozmill.blargon7.com/#/endurance/report/88b31ff45aa5a2c9f30fa1462d02b80d

Comment on attachment 535362 [details] [diff] [review] Updated dashboard to show results from Firefox 6.0. v2.0 >- value.min_allocated_memory = Math.round(min(value.allocated_memory) * BYTE_TO_MEGABYTE); >- value.max_allocated_memory = Math.round(max(value.allocated_memory) * BYTE_TO_MEGABYTE); >+ memory_label = (value.stats.explicit) ? "explicit" : "allocated"; >+ value.min_memory = Math.round(value.stats[memory_label].min * BYTE_TO_MEGABYTE); >+ value.max_memory = Math.round(value.stats[memory_label].max * BYTE_TO_MEGABYTE); This needs the update I mentioned on bug 650382 comment 11. Probably it doesn't apply cleanly anymore. >- var allocatedMemoryResults = []; >- var mappedMemoryResults = []; >- var testAverageAllocatedMemoryResults = []; >- var testAverageMappedMemoryResults = []; > > for (var i=0; i < testCount; i++) { >- var testAllocatedMemoryResults = []; >- var testMappedMemoryResults = []; Those line have already been removed with the patch on bug 650382. Can you please update the patch, so I can better review the remaining changes?

Updated patch with changes from trunk. Also now only shows stats if they exist.

Comment on attachment 536779 [details] [diff] [review] Updated dashboard to show results from Firefox 6.0. v3.0 >+ var testMemory = {}; >+ >+ if (stats_available) { >+ >+ if (tests[i].stats.allocated) { >+ testMemory.allocated = { >+ min : Math.round(tests[i].stats.allocated.min * BYTE_TO_MEGABYTE), >+ max : Math.round(tests[i].stats.allocated.max * BYTE_TO_MEGABYTE), >+ average : Math.round(tests[i].stats.allocated.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (tests[i].stats.mapped) { >+ testMemory.mapped = { >+ min : Math.round(tests[i].stats.mapped.min * BYTE_TO_MEGABYTE), >+ max : Math.round(tests[i].stats.mapped.max * BYTE_TO_MEGABYTE), >+ average : Math.round(tests[i].stats.mapped.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (tests[i].stats.explicit) { >+ testMemory.explicit = { >+ min : Math.round(tests[i].stats.explicit.min * BYTE_TO_MEGABYTE), >+ max : Math.round(tests[i].stats.explicit.max * BYTE_TO_MEGABYTE), >+ average : Math.round(tests[i].stats.explicit.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (tests[i].stats.resident) { >+ testMemory.resident = { >+ min : Math.round(tests[i].stats.resident.min * BYTE_TO_MEGABYTE), >+ max : Math.round(tests[i].stats.resident.max * BYTE_TO_MEGABYTE), >+ average : Math.round(tests[i].stats.resident.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ } >+ Can we move this block to a separate function? >+ if (resp.endurance.stats.allocated) { >+ context.memory.allocated = { >+ min : Math.round(resp.endurance.stats.allocated.min * BYTE_TO_MEGABYTE), >+ max : Math.round(resp.endurance.stats.allocated.max * BYTE_TO_MEGABYTE), >+ average : Math.round(resp.endurance.stats.allocated.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (resp.endurance.stats.mapped) { >+ context.memory.mapped = { >+ min : Math.round(resp.endurance.stats.mapped.min * BYTE_TO_MEGABYTE), >+ max : Math.round(resp.endurance.stats.mapped.max * BYTE_TO_MEGABYTE), >+ average : Math.round(resp.endurance.stats.mapped.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (resp.endurance.stats.explicit) { >+ context.memory.explicit = { >+ min : Math.round(resp.endurance.stats.explicit.min * BYTE_TO_MEGABYTE), >+ max : Math.round(resp.endurance.stats.explicit.max * BYTE_TO_MEGABYTE), >+ average : Math.round(resp.endurance.stats.explicit.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ if (resp.endurance.stats.resident) { >+ context.memory.resident = { >+ min : Math.round(resp.endurance.stats.resident.min * BYTE_TO_MEGABYTE), >+ max : Math.round(resp.endurance.stats.resident.max * BYTE_TO_MEGABYTE), >+ average : Math.round(resp.endurance.stats.resident.average * BYTE_TO_MEGABYTE) >+ } >+ } >+ >+ } ... because we should get rid of this duplicated code. >+++ b/_attachments/templates/endurance_report.mustache >+ {{#stats_available}} >+ {{#memory}} >+ {{#allocated}}<tr> >+ <th>Allocated memory (MB):</th> >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> >+ </tr>{{/allocated}} >+ {{#mapped}}<tr> >+ <th>Mapped memory (MB):</th> >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> >+ </tr>{{/mapped}} >+ {{#explicit}}<tr> >+ <th>Explicit memory (MB):</th> >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> >+ </tr>{{/explicit}} >+ {{#resident}}<tr> >+ <th>Resident memory (MB):</th> >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> >+ </tr>{{/resident}} >+ {{/memory}} >+ {{/stats_available}} Please file a new bug for hiding the non-existent stats. Not sure yet, what's the best way to not show all metrics. Otherwise looks good.

(In reply to comment #8) > Comment on attachment 536779 [details] [diff] [review] [review] > Updated dashboard to show results from Firefox 6.0. v3.0 > > >+ var testMemory = {}; > >+ > >+ if (stats_available) { > >+ > >+ if (tests[i].stats.allocated) { > >+ testMemory.allocated = { > >+ min : Math.round(tests[i].stats.allocated.min * BYTE_TO_MEGABYTE), > >+ max : Math.round(tests[i].stats.allocated.max * BYTE_TO_MEGABYTE), > >+ average : Math.round(tests[i].stats.allocated.average * BYTE_TO_MEGABYTE) > >+ } > >+ } > >+ > >+ if (tests[i].stats.mapped) { > >+ testMemory.mapped = { > >+ min : Math.round(tests[i].stats.mapped.min * BYTE_TO_MEGABYTE), > >+ max : Math.round(tests[i].stats.mapped.max * BYTE_TO_MEGABYTE), > >+ average : Math.round(tests[i].stats.mapped.average * BYTE_TO_MEGABYTE) > >+ } > >+ } > >+ > >+ if (tests[i].stats.explicit) { > >+ testMemory.explicit = { > >+ min : Math.round(tests[i].stats.explicit.min * BYTE_TO_MEGABYTE), > >+ max : Math.round(tests[i].stats.explicit.max * BYTE_TO_MEGABYTE), > >+ average : Math.round(tests[i].stats.explicit.average * BYTE_TO_MEGABYTE) > >+ } > >+ } > >+ > >+ if (tests[i].stats.resident) { > >+ testMemory.resident = { > >+ min : Math.round(tests[i].stats.resident.min * BYTE_TO_MEGABYTE), > >+ max : Math.round(tests[i].stats.resident.max * BYTE_TO_MEGABYTE), > >+ average : Math.round(tests[i].stats.resident.average * BYTE_TO_MEGABYTE) > >+ } > >+ } > >+ > >+ } > >+ > > Can we move this block to a separate function? Done. > > >+++ b/_attachments/templates/endurance_report.mustache > >+ {{#stats_available}} > >+ {{#memory}} > >+ {{#allocated}}<tr> > >+ <th>Allocated memory (MB):</th> > >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> > >+ </tr>{{/allocated}} > >+ {{#mapped}}<tr> > >+ <th>Mapped memory (MB):</th> > >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> > >+ </tr>{{/mapped}} > >+ {{#explicit}}<tr> > >+ <th>Explicit memory (MB):</th> > >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> > >+ </tr>{{/explicit}} > >+ {{#resident}}<tr> > >+ <th>Resident memory (MB):</th> > >+ <td>Minimum: <strong>{{min}}</strong> / Maximum: <strong>{{max}}</strong> / Average: <strong>{{average}}</strong></td> > >+ </tr>{{/resident}} > >+ {{/memory}} > >+ {{/stats_available}} > > Please file a new bug for hiding the non-existent stats. Not sure yet, > what's the best way to not show all metrics. Done. See bug 661495

Comment on attachment 536854 [details] [diff] [review] Updated dashboard to show results from Firefox 6.0. v3.1 Looks good. Please land it and mark as to be pushed. I will do that later.

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

Has been pushed to production.