catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.93k stars 564 forks source link

Dashboard - Handling tir_labels incorrectly on /add_histograms #4380

Closed simonhatch closed 6 years ago

simonhatch commented 6 years ago

https://chromeperf.appspot.com/report?sid=41e8377ec2fcf5a655e4611925dda666f186b087d44426ddeaee4ac1e7b8db2b

Chartjson has both [cold, warm] under timeToFirstContentfulPaint_avg

ChromiumPerfFyi/histogram-diff-chartjson-20/loading.desktop / timeToFirstContentfulPaint_avg / cold

Whereas histograms seems to mash them together for some reason:

ChromiumPerfFyi/histogram-diff-histograms-20/loading.desktop / timeToFirstContentfulPaint / cold_warm

@eakuefner

eakuefner commented 6 years ago

We need to set IS_SUMMARY for all the merged histograms to fix this bug.

eakuefner commented 6 years ago

More discussion offline with @simonhatch -- we don't need this, because there are no summaries across tags in chart JSON. Fixing #4387 will get rid of the histograms that we don't need because they're not present in chart JSON, and will add the histograms that we expect because they are present in chart JSON.

eakuefner commented 6 years ago

This has been fixed by the same fix for #4387