search

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.91k stars 563 forks source link

Dashboard: Add support for bytes-per-second units #4570

Closed mseaborn closed 5 years ago

mseaborn commented 5 years ago

The Catapult dashboard currently supports a limited set of units, documented here: https://github.com/catapult-project/catapult/blob/master/docs/histogram-set-json-format.md. This includes "ms", "sizeInBytes", "J".

I would like to be able to graph values in units of bytes per second -- i.e. data throughput rates. (More specifically, I would like to display units of megabytes per second or mebibytes per second.)

It looks like Catapult whitelists acceptable units. If I try to upload a JSON HistogramSet file containing units of "bytesPerSecond_biggerIsBetter", I get a 500 HTTP error.

What is the reason that the Catapult dashboard whitelists the allowable units? Would it be OK to change it to accept any unit strings?

simonhatch commented 5 years ago

@eakuefner

eakuefner commented 5 years ago

This is something we've been thinking about for a while, and I spoke to @benshayden about an approach we think is reasonable.

The TBMv2 unit system is actually tied to a bunch of frontend display/conversion logic, and units are also used to determine default bin boundaries for histograms. For example, you can see that sizeInBytes specifies a sane unitScale for memory units here https://github.com/catapult-project/catapult/blob/master/tracing/tracing/base/unit.html#L373. This logic is used in trace-viewer, results.html, and the dashboard.

By the way, we're trying to use Monorail these days for dealing with perf dashboard bugs -- you can use Report Issue > Report a Perf Dashboard Bug on the perf dashboard to report bugs conveniently in the future. I'll file a Monorail bug about implementing this and refer to it here once filed.

benshayden commented 5 years ago

This is happening here.

benshayden commented 5 years ago

This should be done. Feel free to try to upload data with the new units and let me know if they don't look right on v2spa.