Closed Bug 766598 Opened 8 years ago Closed 4 years ago

move responsiveness metric to filter.py

Categories

(Testing :: Talos, defect)

defect
Not set

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: k0scist, Assigned: malayaleecoder)

References

Details

(Whiteboard: [talos_wishlist])

Attachments

(1 file, 2 obsolete files)

Currently a responsiveness metric lives in output.py:

http://hg.mozilla.org/build/talos/file/45b903369dca/talos/output.py#l89

    89     def responsiveness_Metric(cls, val_list):
    90         return round(sum([int(x)*int(x) / 1000000.0 for x in val_list]))

This is more like a filter and probably belongs in filter.py . It
would also be nice to document what this is doing (or supposed to be
doing).  I'm not sure looking at it what it is....the sum of squares
divided by some constant....hmmmm....
we should determine if this is possible and would be simple to move.
Whiteboard: [talos_wishlist]
jmaher, shall I give it a try?
Flags: needinfo?(jmaher)
yes, this would be a good one.  for the most part this is just moving code from output.py  -> filter.py and fixing the references to it.

To test this we would need to run responsiveness locally.  Currently it would be simpler to avoid running tp5 (the only test that measures responsiveness) and make another test like tsvgx measure it.  Here is how we turn it on for tp5o:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/test.py#479

I would hack that up so you can |mach talos-test -a tsvgx|, then look at the output and determine if we get responsiveness metric properly.
Assignee: nobody → malayaleecoder
Flags: needinfo?(jmaher)
Attached patch Bug766598_v1.diff (obsolete) — Splinter Review
jmaher, did not include the review flag as it is mostly still WIP.
Comment on attachment 8756014 [details] [diff] [review]
Bug766598_v1.diff

Review of attachment 8756014 [details] [diff] [review]:
-----------------------------------------------------------------

two things to adjust here.

::: testing/talos/talos/filter.py
@@ +187,5 @@
>  
>      return reference[name] / geometric_mean(series)
> +
> +
> +@classmethod

we do not want a @classmethod here

::: testing/talos/talos/output.py
@@ +140,5 @@
>          pass
>  
>      def construct_results(self, vals, testname):
>          if 'responsiveness' in testname:
> +            return self.filter.responsiveness_Metric([val for (val, page) in vals])

I am not sure you need self.filter, filter might be good enough
Attachment #8756014 - Flags: feedback-
Attached patch Bug766598_v2.diff (obsolete) — Splinter Review
Attachment #8756014 - Attachment is obsolete: true
Attachment #8756536 - Flags: review?(jmaher)
Comment on attachment 8756536 [details] [diff] [review]
Bug766598_v2.diff

Review of attachment 8756536 [details] [diff] [review]:
-----------------------------------------------------------------

looks much better
Attachment #8756536 - Flags: review?(jmaher) → review+
Attachment #8756536 - Attachment is obsolete: true
Attachment #8756933 - Flags: review?(jmaher)
Comment on attachment 8756933 [details] [diff] [review]
Bug766598_v3.diff

Review of attachment 8756933 [details] [diff] [review]:
-----------------------------------------------------------------

woohoo, no self/cls definition anymore!
Attachment #8756933 - Flags: review?(jmaher) → review+
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/2870fc2fd1ce
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.