Skip to content

[tool] the load_and_parse_large_asset_manifest benchmark doesn't use the intended example asset manifest #128303

@andrewkolos

Description

@andrewkolos

Discovered while creating #128143.

#118410 changed the code of the load_and_parse_large_asset_manifest benchmark to no longer load and decode money_asset_manifest.json (which represents an asset manifest from a large app with many assets). This is because the framework no longer uses the JSON version of the asset manifest.

Now, the manifest loaded in the benchmark is the one from the defualt rootBundle, which

  1. is not equivalent to money_asset_manifest.json, making the history of the benchmark misleading (unless the numbers just happen to be close to what they would have been were the benchmark loading an equivalent manifest) and
  2. could change (even if slightly) if an asset is added to the microbenchmarks project for an unrelated benchmark change.

The first problem is irreversible, but the second still worth considering. There is also the matter of money_asset_manifest.json sitting in the repo as an unused file.

Addressing the issue

As a first option, we could consider this not to be worth fixing and delete the benchmark or just keep it as it is.

As a second option, the quickest fix would be to generate a new version of money_asset_manifest.json using the same scheme and encoding that we use for the asset manifest today. We then would update the benchmark to load that. However, this would make the bloat the git history of the repo, which is already large (see this comment).

As a third option, we could have a modern, binary version of money_asset_manifest.json generated before building and running the benchmark. I am not aware of a way to do this, and, even if there is a way, it would make the test more complicated.

As a fourth option, we could generate the binary version manifest ahead of time, and have it available as a package asset that comes from some pub package. This would be similar to what we do with flutter_gallery_assets.. However, there is still the question of where do we put this. In a new package? In flutter_gallery_assets (where I feel it would be out-of-place)? This also makes updating the manifest (in case the format/encoding changes again in the future) into more work as two repos would have to be updated.

On a less important note, the last three options would still require us exposing the asset manifest decoding code for testing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Issues that are less important to the Flutter projectteam-toolOwned by Flutter Tool teamtoolAffects the "flutter" command-line tool. See also t: labels.triaged-toolTriaged by Flutter Tool team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions