Skip to content

Add Windows performance benchmark#99564

Merged
fluttergithubbot merged 5 commits intoflutter:masterfrom
jonahwilliams:windows_perf_benchmark
Mar 4, 2022
Merged

Add Windows performance benchmark#99564
fluttergithubbot merged 5 commits intoflutter:masterfrom
jonahwilliams:windows_perf_benchmark

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Mar 4, 2022

We need at least one benchmark to test if flutter/engine#31830 improves rendering performance.

This adds a single windows host benchmark, with a small change to the tool to simplify devicelab integration

@flutter-dashboard flutter-dashboard bot added f: scrolling Viewports, list views, slivers, etc. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Mar 4, 2022
.ci.yaml Outdated
- bin/**
- .ci.yaml

- name: Windows home_scroll_perf__timeline_summary
Copy link
Contributor

Choose a reason for hiding this comment

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

Builder name follows the format Platform + task name: Windows home_scroll_perf__timeline_summary => Windows windows_home_scroll_perf__timeline_summary

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this will fail the led run, will try based on this PR to see how it goes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@jonahwilliams
Copy link
Contributor Author

Updated the tool so it now supports non-fatal --screenshot on unsupported devices

Cache.enableLocking();
});

testUsingContext('warns if screenshot is not supported but continues test', () async {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @christopherfujino , this is a lot easier than tracking whether each device supports screenshotting in the devicelab codebase

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

We also need an entry in ownership file: https://github.com/flutter/flutter/blob/master/TESTOWNERS to bypass the owner check.

.ci.yaml Outdated

- name: Windows windows_home_scroll_perf__timeline_summary
recipe: devicelab/devicelab_drone
presubmit: true
Copy link
Contributor

Choose a reason for hiding this comment

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

The current workflow is using bringup: true first.
We don't need presubmit: true here: 1) ci yaml roller use default value true for presubmit 2) With this set, it will cause the presubmit hang on this test as there is not available builder config yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 4, 2022
@jonahwilliams jonahwilliams marked this pull request as ready for review March 4, 2022 21:08
@jonahwilliams jonahwilliams changed the title add windows benchmark Add Windows performance benchmark Mar 4, 2022
TESTOWNERS Outdated
Comment on lines +77 to +79
## Windows Devicelab tests
/dev/devicelab/bin/tasks/windows_home_scroll_perf__timeline_summary.dart @jonahwilliams @flutter/engine

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this can be added in section: ## Host only DeviceLab tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM re CI config.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

Test LGTM

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

Cache.enableLocking();
});

testUsingContext('warns if screenshot is not supported but continues test', () async {
Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2022
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. f: scrolling Viewports, list views, slivers, etc. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants