Skip to content

Memory benchmark for showing large images in succession - #64762

Merged
dnfield merged 9 commits into
flutter:masterfrom
dnfield:image_page_benchmark
Aug 31, 2020
Merged

dnfield merged 9 commits into
flutter:masterfrom
dnfield:image_page_benchmark

Conversation

@dnfield

@dnfield dnfield commented Aug 27, 2020 •

Copy link
Copy Markdown
Contributor

Benchmark for #56482

This will be improved when the following finish landing:

Current behavior observed is memory growth until a GC is finally performed after multiple iterations of changing the image. Expected behavior will be that a GC is performed at least once after a large image is disposed of.

@flutter-dashboard flutter-dashboard Bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Aug 27, 2020
@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@dnfield dnfield mentioned this pull request Aug 27, 2020
8 of 13 tasks
imageIndex = 0;
} else {
imageIndex += 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: line 33-37 can be simplified as imageIndex = (imageIndex + 1) % 6.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

const String kPackageName = 'com.example.macrobenchmarks';
const String kActivityName = 'com.example.macrobenchmarks.MainActivity';

class LargeImageChangerPerfTest extends MemoryTest {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if more memory info (e.g., peak memory, average, percentile, etc.) from DevToolsMemoryTest (see, e.g., this example) or Timeline (see, e.g., profiling_summarizer and flutter-team-archive/engine#18516) would be helpful here? The DevTools works for Android and the Timeline works for iOS.

@dnfield
dnfield requested a review from liyuqian August 28, 2020 17:59
@dnfield

dnfield commented Aug 28, 2020

Copy link
Copy Markdown
Contributor Author

Updated to use a DevTools memory test.

@jonahwilliams

Copy link
Copy Markdown
Contributor

Is it feasible to measure CPU usage as well?

@dnfield

dnfield commented Aug 28, 2020

Copy link
Copy Markdown
Contributor Author

@liyuqian might have more context on that.

What CPU benchmarks do you think this would capture that we're not already capturing though?

@jonahwilliams

Copy link
Copy Markdown
Contributor

specifically the cost of increased GCs to reduce image memory usage

@dnfield

dnfield commented Aug 28, 2020

Copy link
Copy Markdown
Contributor Author

Added an iOS variant that does CPU as well.

await driver.close();
});

// test('Measure CPU/GPU/Memory', () async {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

commented out code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread dev/devicelab/lib/tasks/perf_tests.dart Outdated
}, onDone: () { observatoryUri.complete(null); });
}, onDone: () {
if (!observatoryUri.isCompleted) {
observatoryUri.complete(null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unless you need the null, this can be observatoryUri.complete()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@liyuqian liyuqian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@liyuqian liyuqian added perf: energy Performance issues related to energy use (power, watts) perf: memory Performance issues related to memory c: performance Relates to speed or footprint issues (see "perf:" labels) labels Aug 29, 2020
@dnfield
dnfield merged commit b5ed913 into flutter:master Aug 31, 2020
@dnfield
dnfield deleted the image_page_benchmark branch August 31, 2020 17:04
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@dnfield dnfield mentioned this pull request Sep 25, 2020
1 task done
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. c: performance Relates to speed or footprint issues (see "perf:" labels) perf: energy Performance issues related to energy use (power, watts) perf: memory Performance issues related to memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants