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.

Sign up for GitHub

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

Jump to bottom

20% regression in platform channels benchmark #138689

Closed
zanderso opened this issue Nov 19, 2023 · 5 comments
Closed

20% regression in platform channels benchmark #138689

zanderso opened this issue Nov 19, 2023 · 5 comments
Assignees
@sstrickl
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) dependency: dart Dart team may need to help us P1 High-priority issues at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@zanderso
Copy link
Member

On the engine roll #138647, SkiaPerf is reporting a ~20% regression in the platform_channel_basic_standard_2host_large metric from the platform_channels_benchmarks benchmark:

https://flutter-flutter-perf.skia.org/e/?begin=1700183293&end=1700312483&keys=X25251bae98644a72a9a678545659f5cc&num_commits=50&request_type=1&xbaroffset=38025

That Engine -> Dart roll has a Dart SDK roll (flutter/engine#48187), and from that roll, the following commit looks like it could be related:

https://dart.googlesource.com/sdk.git/+/904354f108818630c98ef5d019bc0a99e1c6f2fd

cc @gaaclarke @sstrickl @mkustermann
@zanderso zanderso added c: performance Relates to speed or footprint issues (see "perf:" labels) dependency: dart Dart team may need to help us team-engine Owned by Engine team labels Nov 19, 2023
@mkustermann
Copy link
Member

mkustermann commented Nov 20, 2023 •
edited
Loading

We also saw a regression from golem (as well as many improvements). I think @sstrickl is looking into this.

@sstrickl
Copy link
Contributor

Yes, investigating now.

@zanderso
Copy link
Member Author

Yes, Moto G4 is 32-bit.

@jonahwilliams jonahwilliams added P1 High-priority issues at the top of the work list triaged-engine Triaged by Engine team labels Nov 20, 2023
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Nov 21, 2023
@sstrickl
[vm/compiler] On 32-bit archs, no specific inline for index checks.
26911a6 
Replacing index checks with a single GenericCheckBound instruction
currently causes performance regressions on 32-bit ARM. Undo that
optimization there for now, instead inlining the original Dart code
into the indexing operations.

TEST=ci

Issue: flutter/flutter#138689
Change-Id: I7e67966b576a9d1b5f510e17b44f853f23c98a91
Cq-Include-Trybots: luci.dart.try:vm-linux-release-simarm-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-linux-release-simarm_x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337480
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
zanderso added a commit that referenced this issue Nov 27, 2023
@zanderso
Run platform_channels_benchmarks on Pixel 7 Pro
9ee1e9d 
Related to #138689 for 64-bit coverage for Dart VM changes in this area
@zanderso zanderso mentioned this issue Nov 27, 2023
Merged
zanderso added a commit that referenced this issue Nov 28, 2023
@zanderso
Run platform_channels_benchmarks on Pixel 7 Pro (#139092)
e49cc81 
Related to #138689 for 64-bit
coverage for Dart VM changes in this area

cc @aam
@zanderso
Copy link
Member Author

This is resolved

@github-actions GitHub Actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@sstrickl sstrickl

Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) dependency: dart Dart team may need to help us P1 High-priority issues at the top of the work list team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sstrickl @mkustermann @zanderso @jonahwilliams