Commit bb70efc
committed
Fix serialization allocation in .NET Framework and < .NET Core 3.1 (#7884)
## Summary of changes
- Fixes "incorrect" generated code from `TagsList`
- Removes significant additional overhead during serialization
- "Fix" vendored System.Buffers code to avoid the same issue
## Reason for change
The current generated code for `TagsList` produces something like this:
```csharp
private static ReadOnlySpan<byte> DbTypeBytes => new byte[] { 167, 100, 98, 46, 116, 121, 112, 101 };
```
This _looks_ like it's allocating a new `byte[]` with every invocation,
but the compiler actually optimizes this away to be completely
zero-allocation, by embedding the array as part of the dll, and then
simply returning a `ReadOnlySpan` wrapper pointing to this fixed data.
You can see this if you look at the generated IL:
```
.method private hidebysig static specialname valuetype [System.Runtime]System.ReadOnlySpan`1<unsigned int8>
get_DbTypeBytes() cil managed
{
.maxstack 8
// [20 58 - 20 109]
IL_0000: ldsflda int64 '<PrivateImplementationDetails>'::A06A154BE3B860D0B56FA96C93523B732045BA0BCE2FFD4769109575CF1953BF
IL_0005: ldc.i4.8
IL_0006: newobj instance void valuetype [System.Runtime]System.ReadOnlySpan`1<unsigned int8>::.ctor(void*, int32)
IL_000b: ret
} // end of method SqlTags::get_DbTypeBytes
```
However, in .NET Framework, even though we have vendored
`ReadOnlySpan<T>` so we can get some of the benefits (mostly cleaner
code), we _don't_ get these benefits. Which means that the above code
_does_ generate a new array with every invocation:
```
.method private hidebysig static specialname valuetype Datadog.Trace.VendoredMicrosoftCode.System.ReadOnlySpan`1<unsigned int8>
get_DbTypeBytes() cil managed
{
.maxstack 8
// [20 58 - 20 109]
IL_0000: ldc.i4.8
IL_0001: newarr [netstandard]System.Byte
IL_0006: dup
IL_0007: ldtoken field int64 '<PrivateImplementationDetails>'::A06A154BE3B860D0B56FA96C93523B732045BA0BCE2FFD4769109575CF1953BF
IL_000c: call void [netstandard]System.Runtime.CompilerServices.RuntimeHelpers::InitializeArray(class [netstandard]System.Array, valuetype [netstandard]System.RuntimeFieldHandle)
IL_0011: call valuetype Datadog.Trace.VendoredMicrosoftCode.System.ReadOnlySpan`1<!0/*unsigned int8*/> valuetype Datadog.Trace.VendoredMicrosoftCode.System.ReadOnlySpan`1<unsigned int8>::op_Implicit(!0/*unsigned int8*/[])
IL_0016: ret
} // end of method SqlTags::get_DbTypeBytes
```
This is... Bad 😅 And it explains the _significant_ serialization
overhead identified in #7882 for .NET Framework. I also confirmed this
applies to all <.NET Core 3.1 too (because we compile for .NET Standard)
| Method | Runtime | Mean | Allocated | Alloc Ratio |
| -------------------------- | -------------------- | -------: |
--------: | ----------: |
| WriteEnrichedTraces_Before | .NET 6.0 | 488.9 us | 110 B | 0.001 |
| WriteEnrichedTraces_Before | .NET Framework 4.7.2 | 703.3 us | 112537
B | 1.000 |
| | | | | |
| WriteEnrichedTraces_After | .NET 6.0 | 469.1 us | 105 B | 0.50 |
| WriteEnrichedTraces_After | .NET Framework 4.7.2 | 703.4 us | 208 B |
1.00 |
## Implementation details
The fix is to just do what we were doing before #5298 introduced this
regression 😄 i.e. generate code like this:
```csharp
#if NETCOREAPP
private static ReadOnlySpan<byte> DbTypeBytes => new byte[] { 167, 100, 98, 46, 116, 121, 112, 101 };
#else
private static readonly byte[] DbTypeBytes = new byte[] { 167, 100, 98, 46, 116, 121, 112, 101
#endif
```
## Test coverage
This is all covered by existing tests, and the new benchmark shows the
improvment
## Other details
I found a couple of other places in the vendored code that has the same
issue, and fixed them directly in the code. However, this is not ideal,
as if we re-vendor, we'll clobber these updates, so we'll need to update
the vendoring code too1 parent 7b88380 commit bb70efc
File tree
244 files changed
+5677
-10
lines changed- tracer
- src
- Datadog.Trace.SourceGenerators/TagsListGenerator
- Datadog.Trace
- Generated
- net461/Datadog.Trace.SourceGenerators/TagListGenerator
- net6.0/Datadog.Trace.SourceGenerators/TagListGenerator
- netcoreapp3.1/Datadog.Trace.SourceGenerators/TagListGenerator
- netstandard2.0/Datadog.Trace.SourceGenerators/TagListGenerator
- Vendors
- System.Memory/Buffers
- System.Reflection.Metadata/src/System/Reflection/PortableExecutable
- test/Datadog.Trace.SourceGenerators.Tests
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
244 files changed
+5677
-10
lines changedLines changed: 34 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | | - | |
| 91 | + | |
| 92 | + | |
92 | 93 | | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
97 | 109 | | |
98 | 110 | | |
99 | 111 | | |
| |||
112 | 124 | | |
113 | 125 | | |
114 | 126 | | |
115 | | - | |
| 127 | + | |
| 128 | + | |
116 | 129 | | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
121 | 145 | | |
122 | 146 | | |
123 | 147 | | |
| |||
Lines changed: 24 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
| 30 | + | |
22 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
23 | 35 | | |
| 36 | + | |
24 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
25 | 41 | | |
| 42 | + | |
26 | 43 | | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
27 | 47 | | |
| 48 | + | |
28 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
29 | 53 | | |
30 | 54 | | |
31 | 55 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
22 | 30 | | |
23 | 31 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
20 | 24 | | |
21 | 25 | | |
| |||
Lines changed: 16 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
| 30 | + | |
22 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
23 | 35 | | |
| 36 | + | |
24 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
25 | 41 | | |
26 | 42 | | |
27 | 43 | | |
| |||
Lines changed: 12 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
| 30 | + | |
22 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
23 | 35 | | |
24 | 36 | | |
25 | 37 | | |
| |||
Lines changed: 24 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
| 30 | + | |
22 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
23 | 35 | | |
| 36 | + | |
24 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
25 | 41 | | |
| 42 | + | |
26 | 43 | | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
27 | 47 | | |
| 48 | + | |
28 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
29 | 53 | | |
30 | 54 | | |
31 | 55 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
22 | 30 | | |
23 | 31 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
22 | 30 | | |
23 | 31 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
19 | 23 | | |
| 24 | + | |
20 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
21 | 29 | | |
22 | 30 | | |
23 | 31 | | |
| |||
0 commit comments