820777 - talos tsvg fails to record data properly when a manifest has a mix of own timing and pageloader timing

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

tsvg has a problem where gearflowers.svg only reports 1 data point when it should be reporting 3, likewise hixie007.xml is reporting 5 data points when it should be reporting 3.  

NOISE: __start_tp_report
NOISE: _x_x_mozilla_page_load
NOISE: _x_x_mozilla_page_load_details
NOISE: |i|pagename|runs|
NOISE: |0;gearflowers.svg;2537
NOISE: |1;composite-scale.svg;681;541;548
NOISE: |2;composite-scale-opacity.svg;1142;1156;1125
NOISE: |3;composite-scale-rotate.svg;2042;2042;2019
NOISE: |4;composite-scale-rotate-opacity.svg;2435;2431;2459
NOISE: |5;hixie-001.xml;14818;15054;15172
NOISE: |6;hixie-002.xml;14828;15008;15205
NOISE: |7;hixie-003.xml;305253;325679;331091
NOISE: |8;hixie-004.xml;5703;5830;5669
NOISE: |9;hixie-005.xml;25175;25323;25153
NOISE: |10;hixie-006.xml;30446;30432;30371
NOISE: |11;hixie-007.xml;6448;2256;6739;2320;6720
NOISE: __end_tp_report

The problem is in http://hg.mozilla.org/build/talos/file/f2aec5d0a58c/talos/pageloader/chrome/pageloader.js#l426 where we record the time, but don't clear all the global state variables, in this case recordedName.  So the next test that runs will record the results for the previous test.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: reset the recordedName for tests that do own timing (1.0)

this only seems to affect tests which have mixed timing modes in a manifest and svg.manifest is the only manifest we have that has mixed modes.

Comment 2

[Jonathan Eads ( :jeads )]

The same symptom seems to also occur with tp4m

https://datazilla.allizom.org/talos/testdata/raw/Mozilla-Inbound/a8233178dd72?product=Fennec&test_name=Talos%20tp4m

the replicate count varies between 1, 2, 3 depending on what page was run.

Comment 3

[Jeff Hammel]

Comment on attachment 691287 [details] [diff] [review]
reset the recordedName for tests that do own timing (1.0)

good detective work on this one!  wfm

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: solve the >1 cycle problem (2.0)

the original patch fixed a loophole in mixed mode manifests, this patch fixes a hole in numCycles>1 (on desktop we just do one cycle and use pagecycles instead).

This is the original patch + the new one combined!

Comment 5

[Jeff Hammel]

Comment on attachment 691490 [details] [diff] [review]
solve the >1 cycle problem (2.0)

r+ nice :)

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/1eab02b9475e