Increased WriteBuffers starting capacity to 64 bytes.#101790
Increased WriteBuffers starting capacity to 64 bytes.#101790fluttergithubbot merged 4 commits intoflutter:masterfrom
Conversation
flutter-dashboard
bot
added
the
framework
|
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 (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| class WriteBuffer { | ||
| /// Creates an interface for incrementally building a [ByteData] instance. | ||
| factory WriteBuffer() { | ||
| /// [startCapacity] determines the start size of the [WriteBuffer]. The closer |
There was a problem hiding this comment.
nit: initial capacity in bytes. Also assert for >= 0
There was a problem hiding this comment.
I would assert >= 0, 0 is still a valid initial buffer size
|
For testing, how about making sure we can set the capacity and that it throws an assert if < 0 and doesn't if >= 0? |
done |
| @@ -16,6 +16,7 @@ class WriteBuffer { | |||
| /// [startCapacity] determines the start size of the [WriteBuffer]. The closer | |||
There was a problem hiding this comment.
| /// [startCapacity] determines the start size of the [WriteBuffer]. The closer | |
| /// [startCapacity] determines the initial capacity of the [WriteBuffer]. The closer |
| expect(() => write.done(), throwsStateError); | ||
| }); | ||
| test('empty WriteBuffer', () { | ||
| expect(() => WriteBuffer(startCapacity: 0), throwsAssertionError); |
There was a problem hiding this comment.
add expect(() => WriteBuffer(startCapacity: 1), returnsNormally);
gaaclarke
added
waiting for tree to go green
and removed
waiting for tree to go green
labels
|
We should probably do this for the MethodChannel usage of WriteBuffers too. Since it's late I'll just let this one land and I'll make a new PR tomorrow. |
3 participants
By selecting a larger starting buffer size we get a 28% speed increase on
StandardMessageCodec_heterogenous_listand a 13% speed increase onStandardMessageCodec_heterogenous_map.The reason this is faster is that the buffer was so small that we were resizing the buffer a lot and the resizes are exponentially more expensive since they are copying the last region that was copied. By starting at a larger size we are removing that feedback.
It probably has the best performance gains for payloads that are between 8 bytes and 256 bytes. Smaller than that and we won't be resizing, larger than that and the savings becomes insignificant to larger resizing. It didn't noticeably effect payloads <= 8bytes
Do not land until #101767 lands.
This should also be covered by previous tests, it's just a performance change.
before:
after:
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.