Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jonahwilliams
Copy link
Contributor

This benchmark includes things like allocating geometry/contents, path conversions, et cetera. It doesn't include dispatching or GPU work, but should make it easier to see improvements/regressions in the efficiency of this code.

@jonahwilliams jonahwilliams marked this pull request as ready for review November 26, 2023 23:07
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

Comment on lines 29 to 35
size_t DrawLine() {
Canvas canvas;
for (auto i = 0; i < 500; i++) {
canvas.DrawLine({0, 0}, {100, 100}, {.color = Color::DarkKhaki()});
}
return 500;
}
Copy link
Member

Choose a reason for hiding this comment

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

These might get optimized to just returning 500. I think taking in a pointer to a Canvas that will do the DrawLine operations is a safer alternative.

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

template <class... Args>
static void BM_CanvasRecord(benchmark::State& state, Args&&... args) {
auto args_tuple = std::make_tuple(std::move(args)...);
auto test_proc = std::get<decltype(&DrawRect)>(args_tuple);
Copy link
Member

Choose a reason for hiding this comment

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

nit: can you name the type decltype(&DrawRect) instead of assuming that the passed in functions will match the DrawRect signature?

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

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 28, 2023
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #48374 at sha d8c32fc

@auto-submit auto-submit bot merged commit 80964f7 into flutter:main Nov 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 28, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 28, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 28, 2023
flutter/engine@96137d0...3381d3f

2023-11-28 skia-flutter-autoroll@skia.org Roll Skia from 62b4bf2bef5c to 4a74daba343b (1 revision) (flutter/engine#48436)
2023-11-28 skia-flutter-autoroll@skia.org Roll Dart SDK from 8b962b2b9e72 to b58735e626f6 (1 revision) (flutter/engine#48435)
2023-11-28 skia-flutter-autoroll@skia.org Roll Skia from 600986ba305d to 62b4bf2bef5c (1 revision) (flutter/engine#48433)
2023-11-28 jonahwilliams@google.com [Impeller] Add benchmarks that measure the time it takes to record canvas operations. (flutter/engine#48374)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
caseycrogers pushed a commit to caseycrogers/flutter that referenced this pull request Dec 29, 2023
…9119)

flutter/engine@96137d0...3381d3f

2023-11-28 skia-flutter-autoroll@skia.org Roll Skia from 62b4bf2bef5c to 4a74daba343b (1 revision) (flutter/engine#48436)
2023-11-28 skia-flutter-autoroll@skia.org Roll Dart SDK from 8b962b2b9e72 to b58735e626f6 (1 revision) (flutter/engine#48435)
2023-11-28 skia-flutter-autoroll@skia.org Roll Skia from 600986ba305d to 62b4bf2bef5c (1 revision) (flutter/engine#48433)
2023-11-28 jonahwilliams@google.com [Impeller] Add benchmarks that measure the time it takes to record canvas operations. (flutter/engine#48374)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller will affect goldens

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants