Avoid copying the contents of large platform message responses #4947
Conversation
| namespace { | ||
|
|
||
| // Avoid copying the contents of messages beyond a certain size. | ||
| const int kMessageCopyThreshold = 100; |
There was a problem hiding this comment.
How about making this one page sized instead?
There was a problem hiding this comment.
This was based on measurement of the tradeoff between allocating (and later deleting) the new vector versus doing the memcpy.
@rmacnak-google do external objects have any additional performance cost compared to a standard TypedData?
There was a problem hiding this comment.
Yes, the size of the weak handle (~4 words) and the time for the GC to look at the weak handle table to see if they have been collected (a few dozen instructions). I'd expect it to pay off before one page, but even a high threshold here I think is fine because the bulk of the benefit comes from the very large messages.
There was a problem hiding this comment.
Increased the threshold to 1000 bytes
|
This is great. |
Assets are loaded via platform messages, and currently asset payloads are being copied into Dart typed data buffers. This change uses external typed data objects that wrap the existing buffer if copying would be expensive. See flutter/flutter#16291
jason-simmons
force-pushed
the
asset_memcpy
branch
from
3edf24b
to
c454dee
Compare
|
@jason-simmons mentioned the other day that Dart allows modification of typed external data allocations. This would mean that Dart code could be in the middle of modifying the buffer when the engine is attempting to read from it (usually for image decompression). We should make sure those buffers are un-modifiable as mentioned in flutter/flutter#16291 before this patch makes it into an engine roll. |
|
The buffer in the std::vector given to WrapByteData is a copy - it isn't the original buffer returned by the raw resource access API. |
Assets are loaded via platform messages, and currently asset payloads are
being copied into Dart typed data buffers. This change uses external
typed data objects that wrap the existing buffer if copying would be
expensive.
See flutter/flutter#16291