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

postEvent should be a no-op if the event stream has no listeners. #48860

Closed
dnfield opened this issue Apr 21, 2022 · 2 comments
Closed

postEvent should be a no-op if the event stream has no listeners. #48860

dnfield opened this issue Apr 21, 2022 · 2 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter

Comments

@dnfield
Copy link
Contributor

dnfield commented Apr 21, 2022

See also flutter/flutter#102197

Today, developer.postEvent does json encoding even in product mode. It then makes a native call which eventually checks whether the extension_stream is enabled and bails out early there.

It would be good to:

  • Expose whether the extension stream is enabled in dart:developer
  • Avoid doing the JSON encoding if it is disabled
@dnfield dnfield added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter labels Apr 21, 2022
@dnfield dnfield self-assigned this Apr 21, 2022
@dnfield
Copy link
Contributor Author

dnfield commented Apr 21, 2022

This is desirable for Flutter because we have some events that we post every frame (or almost every frame), and would like to avoid that overhead when doing performance benchmarks that have no listeners.

It would also be good to avoid the overhead of JSON encoding in release mode.

@dnfield
Copy link
Contributor Author

dnfield commented Apr 22, 2022

copybara-service bot pushed a commit that referenced this issue Apr 26, 2022
This avoids json encoding that was otherwise happening even in product mode. JSON encoding shows up CPU profiling as taking significant time, particularly on low end devices.

TEST=runtime/observatory/tests/service/developer_extension_test.dart

Bug: #48860
Change-Id: I2cf4d949e85c0b23de01ec2033b04527d40c76fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242081
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Dan Field <dnfield@google.com>
@dnfield dnfield closed this as completed Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter
Projects
None yet
Development

No branches or pull requests

1 participant