Cc'ing perf and perfbot sheriffs.

Simon or Oysteine, can you guys find an owner for this, or someone who can help me fix it? This should be a trivial fix.

+eakuefner

Ethan do you know how to change this?

I've just diagnosed what's happening here:

1. It's not mandatory to upload an improvement direction (improvement_direction in chart JSON, or bigger_is_better in the legacy format that is uploaded by parsing C++ test output, as here, according to https://github.com/catapult-project/catapult/blob/master/dashboard/docs/data-format.md. Indeed angle_perftests do no such thing, because their output is parsed by PerformanceLogProcessor.
2. If you don't upload a unit, we'll look up what the improvement direction should be by looking in a map on the dashboard side. The improvement direction for 'score' is 'up', so this should work.
3. If you look at the data that's being uploaded, the 'score' unit is actually 'score\r'. This points to a bug in PerformanceLogProcessor, namely that it doesn't handle Windows newlines properly.

We can confirm that this is a bug in PerformanceLogProcessor by looking at old Windows data for cc_perftests and observing this same exact issue there, and by looking at current Android data for cc_perftests and not observing this issue there.

I think this points to a quick fix that's needed in PerformanceLogProcessor (namely, we should be calling rstrip when we parse units). I'll pick it up, and hopefully this problem should be fixed automatically after my CL lands and a new set of angle_perftests data is uploaded.

(whoops, s/upload a unit/upload an improvement direction/)

CL: https://codereview.chromium.org/2570593005

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d07e4d61d4e028719202666c839c09629f574ce7

commit d07e4d61d4e028719202666c839c09629f574ce7
Author: eakuefner <eakuefner@chromium.org>
Date: Tue Dec 13 23:48:03 2016

Strip units string in legacy perf dashboard JSON generator

C++ tests that run on Windows upload units that have \r at the end because we
never strip the units field and units happen to be positioned at the very ends
of lines processed by this script.

BUG= 670168 

Review-Url: https://codereview.chromium.org/2570593005
Cr-Commit-Position: refs/heads/master@{#438341}

[modify] https://crrev.com/d07e4d61d4e028719202666c839c09629f574ce7/tools/perf/generate_legacy_perf_dashboard_json.py

This should be fixed. Jamie, feel free to verify.

Thanks! The dashboard linked above (https://chromeperf.appspot.com/report?sid=80db85212ec62dafc4bd3cc3137c470c7bce76d2757a0a22fee3096110014bab&rev=435132) still shows as a regression. Is there any way to fix that?

Likely not, unfortunately; my CL will only improve the behavior for generating new alerts. We could mutate this alert directly in the data store but I'm not sure it's worth it. Probably the best thing to do here is just hit ignore, as long as the improvement/"regression" was intended.

No worries, just wanted to make sure it was expected. Thanks!