Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a benchmark for ImageCache #33814

Merged
merged 3 commits into from Jun 4, 2019

Conversation

iskakaushik
Copy link
Contributor

Description

Benchmarks memory consumed by large images in small containers. This will let us get initial numbers, which we are hoping to improve with: #31164

Related Issues

#26194

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@iskakaushik iskakaushik requested a review from liyuqian June 4, 2019 00:12
@Piinks Piinks added a: images Loading, displaying, rendering images framework flutter/packages/flutter repository. See also f: labels. labels Jun 4, 2019
Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

import 'package:flutter_test/flutter_test.dart';

// Once we provide an option for images to be resized to
// fit the container, we should see a significant drop in
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: two spaces in

a  significant

));

await SchedulerBinding.instance.endOfFrame;
await Future<void>.delayed(const Duration(milliseconds: 50));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this waiting for the GPU thread to rasterize the frame? If so, please add a comment here because this is potentially a flaky test. We should change it once we had more reliable mechanism (e.g., Window.onReportTimings in flutter/engine#8983).

@iskakaushik iskakaushik merged commit 92bfc99 into flutter:master Jun 4, 2019
@iskakaushik iskakaushik deleted the image-cache-benchmark branch June 4, 2019 19:33
kiku-jw pushed a commit to kiku-jw/flutter that referenced this pull request Jun 14, 2019
* Add an image cache benchmark for a monochrome image
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: images Loading, displaying, rendering images framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants