Improve CborWriter buffer growth #92435
Conversation
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-formats-cbor, @bartonjs, @vcsjones Issue DetailsFixes #92425.
|
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.cs
Outdated
Show resolved
Hide resolved
| if (newCapacity < requiredCapacity) | ||
| newCapacity = requiredCapacity; | ||
|
|
||
| byte[] newBuffer = new byte[newCapacity]; |
There was a problem hiding this comment.
would it be worth using GC.AllocateUninitializedArray here?
There was a problem hiding this comment.
I'll let the maintainers decide whether it makes sense here.
There was a problem hiding this comment.
AllocateUninitializedArray would probably be OK, except it's not available for .NET Standard 2.0. I don't think it's important enough here that it's worth a #if, so just always using new byte[newCapacity] seems fine.
There was a problem hiding this comment.
Why not just use Array.Resize as before? Presumably the original buffer is close to full so we're not saving a lot by only copying the written slice.
There was a problem hiding this comment.
Presumably the original buffer is close to full so we're not saving a lot by only copying the written slice.
The intent was to avoid copying empty space here, since copying is really the biggest cost on resizing here.
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.cs
Show resolved
Hide resolved
…er/CborWriter.cs Co-authored-by: Jeremy Barton <jbarton@microsoft.com>
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/Writer/CborWriter.cs
Show resolved
Hide resolved
| @@ -205,11 +205,20 @@ private void EnsureWriteCapacity(int pendingCount) | |||
| throw new OverflowException(); | |||
There was a problem hiding this comment.
Apropos, I wrote a lot of this code and it hindsight it seems like I didn't account for not inlining throws in hot-path methods. We should consider refactoring into throw helpers at some point, though clearly not in scope for this PR.
…er/CborWriter.cs Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
|
@MichalPetryka Can you please fix the merge conflict? |
Done |
Fixes #92425.