This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Avoid creating a vector when constructing Dart typed data objects for platform messages #18838
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… platform messages PlatformMessageResponseDart creates an external typed data object to hold the contents of large platform messages such as loaded assets. The implementation was using a std::vector to hold the message data extracted from an fml::Mapping. The vector is initialized during construction, which is unnecessary given that the data will be immediately overwritten. This change centralizes creation of these typed data objects into Tonic's DartByteData::Create. DartByteData::Create will allocate an external typed data based on a malloc buffer if the size exceeds a threshold. Fixes flutter/flutter#58572
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. |
liyuqian
added a commit
to liyuqian/engine
that referenced
this pull request
Jun 9, 2020
We first add some PlatformMessageResponseDartComplete related benchmarks to test flutter#18838. Specifically, BM_PlatformMessageResponseDartCompleteOuter should improve from ~8897 us to ~1944 us after that PR on my MacBook Pro.
Merged
liyuqian
approved these changes
Jun 9, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Let's land this after #18945 to have some benchmark numbers before and after this PR?
liyuqian
added a commit
to liyuqian/engine
that referenced
this pull request
Jun 10, 2020
We first add a PlatformMessageResponseDartComplete benchmark to test flutter#18838. Specifically, it improves from ~7600 us to ~1200 us after that PR on my MacBook Pro.
liyuqian
added a commit
to liyuqian/engine
that referenced
this pull request
Jun 10, 2020
We first add a PlatformMessageResponseDartComplete benchmark to test flutter#18838. Specifically, it improves from ~7600 us to ~1200 us after that PR on my MacBook Pro.
liyuqian
added a commit
to liyuqian/engine
that referenced
this pull request
Jun 10, 2020
We first add a PlatformMessageResponseDartComplete benchmark to test flutter#18838. Specifically, it improves from ~7600 us to ~1200 us after that PR on my MacBook Pro.
liyuqian
added a commit
to liyuqian/engine
that referenced
this pull request
Jun 10, 2020
We first add a PlatformMessageResponseDartComplete benchmark to test flutter#18838. Specifically, it improves from ~7600 us to ~1200 us after that PR on my MacBook Pro.
@jason-simmons @liyuqian Is this PR ready to merge? |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 18, 2020
…ects for platform messages (flutter/engine#18838)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 18, 2020
…ects for platform messages (flutter/engine#18838)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 18, 2020
…ects for platform messages (flutter/engine#18838)
This was referenced Jul 24, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
cla: yes
perf: speed
Performance issues related to (mostly rendering) speed
severe: performance
Relates to speed or footprint issues.
waiting for tree to go green
This PR is approved and tested, but waiting for the tree to be green to land.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PlatformMessageResponseDart creates an external typed data object to
hold the contents of large platform messages such as loaded assets.
The implementation was using a std::vector to hold the message data
extracted from an fml::Mapping. The vector is initialized during
construction, which is unnecessary given that the data will be
immediately overwritten.
This change centralizes creation of these typed data objects into
Tonic's DartByteData::Create. DartByteData::Create will allocate an
external typed data based on a malloc buffer if the size exceeds a
threshold.
Fixes flutter/flutter#58572