Closed Bug 657508 Opened 8 years ago Closed 8 years ago

Update dashboard to display endurance tests results from Firefox 6.0

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: davehunt, Assigned: davehunt)

References

Details

Attachments

(1 file, 3 obsolete files)

The fix for bug 656869 requires some changes in order for the dashboard to display the results.
Initial patch submitted for feedback.
Assignee: nobody → dave.hunt
Attachment #532785 - Flags: feedback?(anthony.s.hughes)
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?
Attachment #532785 - Flags: feedback?(anthony.s.hughes) → feedback+
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.
Attachment #532785 - Flags: review?(hskupin)
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
Attachment #532785 - Attachment is obsolete: true
Attachment #532785 - Flags: review?(hskupin)
Attachment #535362 - Flags: feedback?(hskupin)
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?
Attachment #535362 - Flags: feedback?(hskupin) → feedback-
Updated patch with changes from trunk. Also now only shows stats if they exist.
Attachment #536779 - Flags: feedback?(hskupin)
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.
Attachment #536779 - Flags: feedback?(hskupin) → feedback+
(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
Attachment #536779 - Attachment is obsolete: true
Attachment #536854 - Flags: review?(hskupin)
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.
Attachment #536854 - Flags: review?(hskupin) → review+
Has been pushed to production.
Keywords: push-needed
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.