Engine benchmarks are no longer running after monorepo

There are a number of benchmarks in the engine (part of the mono)repo that used to be run and recorded in Skia Perf, but they haven't run since December 2024 which is when we switched to the new monorepo.

Here is a graph of one of those benchmarks showing the data points ending on December 24, 2024:

https://flutter-engine-perf.luci.app/e/?begin=1733960713&enable_chart_tooltip=true&end=1735025779&plotSummary=true&queries=test%3DBM_Concat_Perspective_ScaleTranslate_SkM44%26test%3DBM_Concat_Perspective_ScaleTranslate_ImpellerMatrix&use_titles=true

@flar

@flar Thanks for reporting this. This looks related to engine benchmarking/CI infra after the monorepo migration. Labeling for infra and engine team to investigate further.

Engine benchmarks are being run - and uploaded to the GCS buckets for flutter-infra-staging. We confirmed yesterday. Skia's PERF is not processing these though.

After a bit of investigation, this appears to be an issue that the upload code for the engine benchmarks is still using the engine repo as the target repo, but with the commit hash that now comes from the flutter repo. Since those are incompatible, it is likely that the data just bounces off of the Skia upload mechanism.

This might be as simple as updating the repo name, but we need to figure out other issues such as what happens to our ability to search results in the flutter skia perf database if we update the repo name without any further changes (the flutter repo benchmarks use a "task_name + sub_result_name" breakdown of the data whereas the engine repo just used "google benchmark name" as the data point.

@flar

A concrete case of what this costs: a Skia change that made SkParagraph layout about 20% slower
in txt_benchmarks (SkParagraphFixture/LongLayout, measured locally) was in Flutter master from
Oct 29 (Skia roll #195593) to Nov 5 (#196017, fixed in
https://skia-review.googlesource.com/c/skia/+/1414543). None of the framework benchmarks on
flutter-flutter-perf showed a step at that roll; the engine dashboard has no data for that range.

I can confirm @flar's diagnosis, with the exact places:

engine/src/flutter/testing/benchmark/bin/parse_and_send.dart takes the revision from
git log -1 in engine/src/flutter, which is now a flutter/flutter commit.
It wraps every point in FlutterEngineMetricPoint (package metrics_center,
lib/src/flutter.dart), which hard-codes gitRepo: flutter/engine. So files still go to
gs://flutter-skia-perf-prod/flutter-engine/....
The Perf instance config (perf/configs/spanner/flutter-engine.json in skia buildbot) still has
git_repo_config.url = https://github.com/flutter/engine. Ingestion cannot find a flutter/flutter
hash in that repo and drops the file. The upload step itself stays green, e.g.
https://ci.chromium.org/b/8665965319856638017 on 4b077ad.

So the job runs and uploads; only the Perf side throws the data away. Two small fixes:

A. Uploader side (flutter/flutter only): in parse_and_send.dart emit plain MetricPoints with
gitRepo: flutter/flutter. Data lands in flutter-flutter-perf next to the framework
benchmarks. As @flar said, the trace keys look different there (Google Benchmark name vs.
task + sub_result), and the old history stays on flutter-engine-perf.

B. Perf side (one line in skia buildbot): point flutter-engine.json at
https://github.com/flutter/flutter. Trace keys and the dashboard stay the same. I have not
checked how Perf handles the commit-number change for the old data; it may need a re-ingest.
This one needs the Perf team.

B keeps the existing graphs and alerts, so I would prefer it if the Perf team agrees.

So the job runs and uploads; only the Perf side throws the data away. Two small fixes:

A. Uploader side (flutter/flutter only): in parse_and_send.dart emit plain MetricPoints with gitRepo: flutter/flutter. Data lands in flutter-flutter-perf next to the framework benchmarks. As @flar said, the trace keys look different there (Google Benchmark name vs. task + sub_result), and the old history stays on flutter-engine-perf.

B. Perf side (one line in skia buildbot): point flutter-engine.json at https://github.com/flutter/flutter. Trace keys and the dashboard stay the same. I have not checked how Perf handles the commit-number change for the old data; it may need a re-ingest. This one needs the Perf team.

B keeps the existing graphs and alerts, so I would prefer it if the Perf team agrees.

Another factor to consider is that we've added new benchmarks that don't use the same units for all results. According to the tool that verifies file formats it doesn't accept such files (tested by using the sample benchmark jsons and modifying them to have different units), but then again, the same tool does not accept any of the files that we upload.

I've been meaning to diagnose this issue by creating a local Skia Perf instance and checkout our jsons there before I turn the uploads back on, but the instructions to run a local Skia Perf instance aren't working for me.

@flar

Hi @flar — you do not need a local Skia Perf instance before turning them back on. The JSON files we upload are already valid for Skia Perf ingestion:

Why perf-tool ingest validate rejects our files: perf-tool ingest validate (perf/go/ingest/format/format.go -> Validate) checks only Format v1 ("version": 1, "git_hash", "results": [...] against formatSchema.json). Flutter's metrics_center (SkiaPerfPoint.toSkiaPerfJson in lib/src/skiaperf.dart) writes the legacy nanobench BenchData format ("gitHash" and "results": {"<test>": {"default": {...}}}, with no "version" field), so Validate rejects every metrics_center file with "File has unknown format version". (In Format v1 itself, different units work when each metric is its own entry in results with "units" inside result.key; SingleMeasurement sets additionalProperties: false, so adding a unit field per measurement inside one measurements list fails schema validation.)
Why live Skia Perf ingestion accepts them (including mixed units): the ingestion parser (Parser.Parse in perf/go/ingest/parser/parser.go) accepts both Format v1 and legacy BenchData. When "version" is absent, it falls back to getParamsAndValuesFromLegacyFormat, walks every numeric field under results[test]["default"] (skipping "options" and "samples"), and creates one trace per sub_result. https://flutter-flutter-perf.luci.app ingests this same metrics_center legacy format every day.
One small metrics_center quirk with mixed units inside a single benchmark: different benchmarks in the same file have separate results[test] entries, so each keeps its own options. Within a single benchmark, SkiaPerfPoint.toSkiaPerfJson groups all subResults under one "default" map with one shared "options" object (GoogleBenchmarkParser sets unit: <time_unit> only on keys ending in time). In CI (upload_metrics.sh runs dart bin/parse_and_send.dart without --enable-asserts), upload succeeds and Skia Perf ingests every numeric subResult; "options" simply keeps the first subResult's tags. Running parse_and_send.dart with --enable-asserts hits an assert in skiaperf.dart when two subResults of the same benchmark have different options, so that only affects debug/test runs, not ingesting the files already in GCS.

Separately, and completely independent of turning engine benchmarks back on: on the Skia side I recently re-enabled the SkParagraph benchmarks in nanobench (https://skia-review.googlesource.com/c/skia/+/1414546) and am cleaning up and expanding that suite. I will compare our nanobench scenarios against txt_benchmarks myself and send you the comparison afterwards — no need to wait on that here.

@Rusino

@Rusino this is great information! Thanks, I'll look into turning them back on "as is".

If there is a problem with the new metrics we upload, is there a way to purge the old data and replace it with new data that is better "formatted" or "recorded"?

Would there be value in having us migrate to v1 format in the future?

Also, can v1 be taught to honor both a default units and also having some measurements override that unit on a case-by-case basis rather than requiring them to all have their own units? Most of our benchmarks have the same units across all of the tests, and only a few have different units. On the other hand, just having all results include their own metric isn't a huge space cost.

@flar

@flar thanks! One important caveat before you turn them on.

Turning the uploads back on "as is" is not enough. The instance config (perf/configs/spanner/flutter-engine.json) still has git_repo_config.url = https://github.com/flutter/engine, so files with flutter/flutter hashes will still be dropped, now for a reason unrelated to the format. Turning uploads on needs fix A or B from my earlier comment as well.

The missing history may be recoverable, but only with B. metrics_center stores files in GCS under the commit date, and perf-tool ingest force-reingest (run by the Perf team) can re-ingest files for a time range. This only works for files that were actually uploaded to gs://flutter-skia-perf-prod/flutter-engine/. The Upload metrics step in CI is green (e.g. https://ci.chromium.org/b/8665965319856638017 on Nov 5), but you mention turning uploads back on: were they disabled somewhere, and if so, since when? Days with uploads off cannot be recovered this way. I have also not checked how Perf handles the commit-number change for the old data once the URL changes.

Short answers to your questions:

Purging old data. There is no self-serve purge: perf-tool has no delete command, so only the Perf team can remove data. What you can do yourself depends on the trace keys. If the keys stay the same (only values change), a later file for the same commit replaces the old values. If the keys change (renamed tests, different units in the key, or a move to v1), the new data goes into new traces, and the old ones stay visible next to them; they just stop getting new points.
Moving to v1. It has real value: perf-tool ingest validate works, and units are explicit. But the trace keys change, which breaks history and means updating alerts. I would only do it together with option A (which changes the keys anyway), so history breaks once. It is not needed to turn things back on now.
Default units with per-result override. v1 already does this. The file-level key is copied, then each result's key is applied on top and overwrites matching keys (parser.go merges them with paramtools.Params.Add). So you can set "units": "ns" once at the file level and set "units" only on the few results that differ. The one limit: a single entry inside one measurements list cannot have its own unit; such measurements need their own entry in results.

How I checked this. I read the current Skia Perf source and the in-tree metrics_center (1.0.6) the engine uses, then ran the real tools on small sample files:

perf-tool ingest validate --verbose (built from go.skia.org/infra main today) on a v1 file with "units": "ns" in the file-level key and "units": "bytes" in one result's key:

,arch=x86_64,sub_result=cpu_time,test=SkParagraphFixture_LongLayout,units=ns, = 16560
,arch=x86_64,sub_result=cpu_time,test=SkParagraphFixture_ShortLayout,units=ns, = 910
,arch=x86_64,sub_result=bytes_per_op,test=DisplayListBuilder_Ops,units=bytes, = 48

Putting "units" inside a measurements entry fails with Additional property units is not allowed.

A legacy file written by the in-tree metrics_center (GoogleBenchmarkParser + SkiaPerfPoint.toSkiaPerfJson) from a Google Benchmark JSON where one benchmark has cpu_time in ns plus a bytes_per_op counter. Skia Perf's ingestion parser (parser.Parse) accepts it and creates all five traces. It also shows the quirk from my last comment: bytes_per_op gets unit=ns, because the benchmark's cpu_time sets the shared options. With --enable-asserts, the same conversion fails on the options assert in skiaperf.dart.

A correction to my last comment: on these files perf-tool ingest validate fails earlier than I said, with json: cannot unmarshal object into Go struct field Format.results of type []format.Result (legacy results is an object, v1 expects an array), not with File has unknown format version. The conclusion is the same.

I could not check the GCS bucket contents (no access) or run force-reingest.

Uploading to a GCS bucket were never "turned off", they were just being uploaded to the wrong location. When I described turning them back on, what I meant was to start uploading them to the same location that the main repo benchmarks were (and are still) being uploaded to. That also meant copying over the data from the old location to the new location.

So, "turning them back on" was short-hand for "reworking them in a way where they'd actually appear in a Skia Perf dashboard somewhere rather than just being zombied into a dead cloud location"...

One thing I'd like to fix when I address this is that we have several gbench json files that we massage into a json format for skia perf and then we upload them to a cloud location. While each came from a different local json file, the upload process tries to put them all into the same json file in the cloud. I don't know whether this behavior is due to a past inability to load multiple result files in Skia Perf, but it seems to be capable of that now so I want to simplify this and have each uploaded from a local json file into a separate file in the cloud.

Not only would this make the storage more granular, but it would avoid an unnecessary and seemingly convoluted "append" operation as each source json file is digested and converted.

@flar

@flar thanks, that makes sense. Since the files kept being uploaded, the missing history can be recovered in full. Three notes:

Copy only the files uploaded after the monorepo switch. Those have flutter/flutter hashes, and the flutter-flutter instance (git_repo_config.url = https://github.com/flutter/flutter) can resolve them. Older files have flutter/engine hashes, and those commits are not in flutter/flutter's master history: I checked two engine commits from late 2024 with the GitHub compare API, and both come back diverged. Such files would be dropped silently. The older history stays visible on flutter-engine-perf anyway. I have not checked whether copying files into the bucket triggers ingestion on its own; if not, the Perf team can run perf-tool ingest force-reingest for the range.
A correction to my last comment. I said option A changes the trace keys anyway. That is wrong: SkiaPerfPoint.fromPoint drops the repo tag from the options, so the keys stay the same with A, and with the copy the history stays continuous. So moving to v1 would be a separate history break. If you want v1, the easiest time is during the copy: convert the old files to v1 as well, so the keys change only once.
One cloud file per gbench JSON sounds right. The "append" comes from parse_and_send.dart passing the same task name, 'flutter_engine_benchmark', to destination.update(...). metrics_center builds the object name as <repo>/<YYYY/MM/DD/HH>/<rev>/<taskName>_values.json, so all nine runs hit the same object, and SkiaPerfDestination.update takes a GCS lock, reads the file, merges the points, and writes it back each time. Deriving the task name from the source file name (e.g. txt_benchmarks) gives separate objects with no lock and no merge. Perf ingests every .json under the directory, and the trace keys do not change, since the task name only goes into the object name.

Navigation Menu

Repository navigation

Engine benchmarks are no longer running after monorepo #185368

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Engine benchmarks are no longer running after monorepo #185368

Description

Activity

saurabh-mirajkar commented on Apr 22, 2026

jtmcdole commented on Apr 28, 2026

flar commented on May 5, 2026

Rusino commented on Nov 5, 2026

flar commented on Nov 5, 2026

Rusino commented on Nov 5, 2026

flar commented on Nov 6, 2026

flar commented on Nov 6, 2026

Rusino commented on Nov 6, 2026

flar commented on Nov 7, 2026

flar commented on Nov 7, 2026

Rusino commented on Nov 7, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions