Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

Adds a benchmark that can be used to show improvements for flutter/engine#32999

@flutter-dashboard flutter-dashboard bot added a: animation Animation APIs f: gestures flutter/packages/flutter/gestures repository. c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 11, 2022
@jonahwilliams
Copy link
Contributor Author

I need to run update-packages --force-upgrade but holding off

@jonahwilliams jonahwilliams requested a review from dnfield May 11, 2022 03:44
await Future.wait(<Future<ui.ImmutableBuffer>>[
for (String asset in assets)
rootBundle.load(asset).then((ByteData data) {
return ui.ImmutableBuffer.fromUint8List(data.buffer.asUint8List());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is once the ImmutableBuffer.fromAsset change lands that'll be added here right?

I don't know if it makes sense to do here or to do in a separate benchmark, but WDYT of having something measuring the time it takes to instantiateImageCodec vs the new API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i would just replace this code and we'd see the skia perf benchmark numbers change all at once.

I don't know if it makes sense to do here or to do in a separate benchmark, but WDYT of having something measuring the time it takes to instantiateImageCodec vs the new API?

Assuming you already have the bytes loaded?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to suggest a measurement for getting the immutable buffer, and a measurement for getting an image codec from the buffer.

Your change will eliminate copies on both ends, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, on one end. I think calling ui.ImmutableBuffer.fromUint8List includes the copy though. The codec doesn't have any more copies that we can eliminate IIRC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh I see now, you're already capturing that by calling ImmutableBuffer.fromUint8List. Ok.

@fluttergithubbot fluttergithubbot merged commit c1b909e into flutter:master May 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 11, 2022
@jonahwilliams jonahwilliams deleted the add_image_load_benchmark branch May 11, 2022 15:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: animation Animation APIs c: contributor-productivity Team-specific productivity, code health, technical debt. f: gestures flutter/packages/flutter/gestures repository.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants