Jeff Hammel Reporter
	Description • 12 years ago

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....

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 1 • 8 years ago

we should determine if this is possible and would be simple to move.

	Vishnu (:malayaleecoder) Assignee
	Comment 2 • 8 years ago

jmaher, shall I give it a try?

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 3 • 8 years ago

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.

	Vishnu (:malayaleecoder) Assignee
	Comment 4 • 8 years ago

jmaher, did not include the review flag as it is mostly still WIP.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 5 • 8 years ago

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

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 7 • 8 years ago

Comment on attachment 8756536 [details] [diff] [review]
Bug766598_v2.diff

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

looks much better

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 8 • 8 years ago

pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=895557df0e0f

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 10 • 8 years ago

Comment on attachment 8756933 [details] [diff] [review]
Bug766598_v3.diff

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

woohoo, no self/cls definition anymore!

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 11 • 8 years ago

new try push up here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4c891261e4f

	Pulsebot
	Comment 12 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2870fc2fd1ce

	Carsten Book [:Tomcat]
	Comment 13 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/2870fc2fd1ce