-
Notifications
You must be signed in to change notification settings - Fork 337
Add support for BufferWriter<T> to the JsonWriter #2366
Changes from all commits
9079487
cc02f67
b5a0e9a
ca85742
77e1e93
e3cf107
90fccdf
ff445a4
6826845
79bdb15
77e4098
ce52b29
9ba1474
e781ba3
a2008a3
abbb65a
cd3964d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,17 @@ private static ArgumentException GetArgumentException(string message) | |
| return new ArgumentException(message); | ||
| } | ||
|
|
||
| public static void ThrowArgumentExceptionInvalidUtf8String() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this beneficial? Isn't
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The il and asm are larger as it has to load the string reference before calling the Throw helper Via String G_M53912_IG05:
48B9683016EFC1010000 mov rcx, 0x1C1EF163068
488B09 mov rcx, gword ptr [rcx]
E81FFCFFFF call JsonThrowHelper:ThrowArgumentException(ref)
CC int3 Via Enum is smaller as its just passing a numeric const G_M12337_IG05:
33C9 xor ecx, ecx
E8F2FBFFFF call JsonThrowHelper:ThrowArgumentException(int)
CC int3 No Params is just a call G_M36770_IG05:
E8D4FBFFFF call JsonThrowHelper:ThrowArgumentExceptionInvalidUtf8String()
CC int3
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g. if you are inlining it all over the place, or duplicating it due to generics, better to be smaller |
||
| { | ||
| throw GetArgumentExceptionInvalidUtf8String(); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| private static ArgumentException GetArgumentExceptionInvalidUtf8String() | ||
| { | ||
| return new ArgumentException("Invalid or incomplete UTF-8 string"); | ||
| } | ||
|
|
||
| public static void ThrowFormatException() | ||
| { | ||
| throw GetFormatException(); | ||
|
|
||
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.
What is the benefit of adding this attribute?
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.
Rearranges and packs the fields in the most efficient layout; rather than the order you define them (except child structs, which are on pointer boundaries).
I'd always assumed it was the default layout as it is for classes; and you had to specify
[StructLayout(LayoutKind.Sequential)]if you wanted that but seem to not always be the case https://blogs.msdn.microsoft.com/seteplia/2017/09/21/managed-object-internals-part-4-fields-layout/Not sure it would make much difference; due to the object reference (forces Auto); but was trying to find where the drop was (think its now the issue I opened)
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.
I haven't observed any difference in benchmarks when marking the struct layout as auto versus leaving it as default.
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.
Yeah, from the article it seems if there is a GC ref then it will switch to auto; and I think a Span or ArrayFormatter will count as that, so it shouldn't make any difference. Was just hoping...