move and clean-up web_benchmarks package to flutter/packages

[yjbanov]

Move the web benchmark code into a shared package to be used for Flutter benchmarks, Gallery benchmarks, as well as customer benchmarks in the future.

Original sources:

• https://github.com/flutter/flutter/blob/master/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart
• https://github.com/flutter/flutter/blob/master/dev/benchmarks/macrobenchmarks/lib/web_benchmarks.dart
• https://github.com/flutter/flutter/blob/master/dev/devicelab/lib/tasks/web_benchmarks.dart
• https://github.com/flutter/flutter/blob/master/dev/devicelab/lib/framework/browser.dart

This was adapted from the version staged in https://github.com/pennzht/web_benchmarks

Bonus changes

• Change the Dockerfile to use cirrusci/flutter:stable-web image that includes Chrome dependencies.
• Use compute credits for Linux builds so that builds are scheduled promptly.
• Add -x to the build script for better debuggability on CI.

[pennzht]

This looks good! I'll test it with the Gallery either later today or Monday.

[pennzht]

event.params['value'] seems to be already of type Map<String, dynamic>. Why do we need to cast here?

[yjbanov]

While it's true that the list contains Map<String, dynamic> the reified type of the list is List<dynamic>, so we have to cast the element type of the list from dynamic to Map<String, dynamic>.

[pennzht]

The output format has changed. The new format is

[
  {
    "name": "scroll.html.preroll_frame.average",
    "value": 78.34693877551021
  },
  {
    "name": "scroll.html.preroll_frame.outlierAverage",
    "value": 847.5
  },
  {
    "name": "scroll.html.preroll_frame.outlierRatio",
    "value": 10.817270122427715
  },
  ...
]

The old format looks like

{
  "success": true,
  "data": {
    "scroll.html.preroll_frame.average": 93.88659793814433,
    "scroll.html.preroll_frame.outlierAverage": 1061.3333333333333,
    "scroll.html.preroll_frame.outlierRatio": 11.304417847077339,
    ...
  },
  "benchmarkScoreKeys": [
    "scroll.html.drawFrameDuration.average",
    "scroll.html.drawFrameDuration.outlierRatio",
    "scroll.html.totalUiFrame.average"
  ]
}

• Why is the new format preferred?
• Will it break any existing parts of the toolchain?
• How can we return a benchmark result representing an unsuccessful test? (Similar to the previous TaskResult.failure)
• How can we specify which parts are benchmarkScoreKeys?

[yjbanov]

This is intentional. The old format was specific to devicelab. The devicelab expects information, such as whether the run was a success or not, encoded in the JSON message we send to it. The data is separated from benchmarkScoreKeys because the devicelab runs tests other than benchmarks, and so the results frequently contain data other than benchmark scores.

Since this is purely a benchmark package all we need to encode is benchmark scores, so we don't need to separate data from score keys. A failure is also easily expressible by simply throwing an exception and exiting the process with a non-zero exit code. So I simplified the output format by removing unnecessary features.

When we run this code in the devicelab, we will adapt it to the devicelab protocol (which is an internal implementation detail, and can change any time; it's not a public API). It's better to keep this package protocol-agnostic.

[yjbanov]

You reminded me about the term "score". I've been struggling what to call the class I ended up calling BenchmarkMetric. I think a better name is BenchmarkScore. I'll rename.

[mdebbar]

This link doesn't work btw. There has to be a branch or sha after /tree/.

[yjbanov]

It's because it's pointing at itself. It will work once the PR is submitted.

[mdebbar]

I know this is a new file so it won't work. But I tried with an existing file, still didn't work. You have to tell it which tree to look at :)

[yjbanov]

Oh, right. Not sure where I got that "/tree/" from. Must be some copypasta. Done.

[mdebbar]

Github link to the file?

[yjbanov]

Actually, now that this is the same package, I can move shared constants into a common library.

[yjbanov]

Also, ironically, we didn't keep the two in sync. One was 10, the other 100 lol :)

[yjbanov]

Done.

[pennzht]

Thank you! Looks great to me. I ran the Gallery benchmarks using this package; they were successful.

[mdebbar]

Could be done with a single listener on the container, then check the event target to figure out which button was clicked. This would be better for performance, but I don't think it matters much here.

[yjbanov]

Yeah, this part is not mission-critical.

[mdebbar]

Suggestion - replace all three lines with:

html.document.body.innerHtml = '<h2>${profile.name}</h2>';

[yjbanov]

Done.

[mdebbar]

I was going to suggest something like this:

try {
  return html.HttpRequest.request(url, ...);
} on html.ProgressEvent catch(e) {
  if (xhr.status! == 404) {
    return xhr;
  }
  rethrow;
}

but then realized inside the catch there's no way to get the xhr object :(

[yjbanov]

I cleaned up this method. It's only used once and a lot of cases it's trying to handle don't exist.