System.Reflection.Metadata pinning can generate extensive GC heap fragmentation

[noahfalk]

System.Reflection.Metadata uses pinned memory buffers to store the contents of embedded PDBs. When the runtime loads and caches these readers to augment stack traces with source line info it winds up preserving the pinned memory for the lifetime of the app. Long lived pinned objects can generate large GC heap fragmentation leading applications to use far more committed VM than they otherwise would have. We have another internal team at MS observing 132MB of VM wasted due to this pin. The amount of wasted VM has nothing to do with the size the pinned object, it is solely based on where the allocated object happens to fall within the global bounds of the GC heap which varies depending on the overall allocation behavior of all the code in the app.

Although the ideal solution is not to use pinning at all, a likely lower effort solution is to allocate the byte array using the pinned heap on .NET 5 and up where it is available. If fixing this proves not to be viable then the runtime will probably need to pursue alternate solutions such as adding an app configuration switch so that app developers can disable portable PDB usage.

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

---

System.Reflection.Metadata uses pinned memory buffers to store the contents of embedded PDBs. When the runtime loads and caches these readers to augment stack traces with source line info it winds up preserving the pinned memory for the lifetime of the app. Long lived pinned objects can generate large GC heap fragmentation leading applications to use far more committed VM than they otherwise would have. We have another internal team at MS observing 132MB of VM wasted due to this pin. The amount of wasted VM has nothing to do with the size the pinned object, it is solely based on where the allocated object happens to fall within the global bounds of the GC heap which varies depending on the overall allocation behavior of all the code in the app.

Although the ideal solution is not to use pinning at all, a likely lower effort solution is to allocate the byte array using the pinned heap on .NET 5 and up where it is available. If fixing this proves not to be viable then the runtime will probably need to pursue alternate solutions such as adding an app configuration switch so that app developers can disable portable PDB usage.

Author:	noahfalk
Assignees:	tmat
Labels:	area-GC-coreclr, tenet-performance, untriaged
Milestone:	-

[Maoni0]

Although the ideal solution is not to use pinning at all,

does it need to be pinned? what was the reason this was pinned in the first place? we generally only advise POH to be used to replace mandatory pinning usage.

[brianrob]

If it does need to be pinned, does it need to be pinned for this long? From what I can see in the memory dumps that led to this issue is that the object is likely pinned for its lifetime.

[tmat]

The MetadataReader uses pointers for performance, hence pinning is needed while metadata is read.

We essentially implemented ReadOnlySpan<byte> and ReadOnlyMemory<byte> before it existed in the runtime.
https://github.com/dotnet/runtime/tree/main/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks

It might be possible to replace the memory abstractions with ReadOnlySpan<byte> and ReadOnlyMemory<byte>, but SRM is also available for .NET Framework so it might be too complicated to maintain both versions.

[brianrob]

@tmat, do you think it's possible to pin/unpin rather than putting this in the POH?

[Maoni0]

also how frequently would this object be created? are we talking about one object for the lifetime of the process or would one get created every time something happens?

[tmat]

So, this is what happens:

1. PDB content is requested - e.g. someone called Exception.ToString or StackTrace(fRequireSources: true), the runtime enumerates the frames and for those that are in assemblies with PDB embedded calls to SRM to load the PDB data. The resulting reader is cached. The entries in the cache are not evicted AFAIK. This is controlled by code in System.Diagnostics.
2. Upon request SRM allocates an array for the PDB data and stream-reads the compressed Embedded PDB data from the loaded PE image through DeflateStream into the array.
3. The array is then pinned so that the MetadataReader can read from it.

[brianrob]

Is the MetadataReader exposed, or is it an internal implementation detail? I'm wondering if we can't just pin the buffer that contains the PDB when it is in-use. Every time a call comes in, we pin the buffer, get a pointer to it, and then pass it to the MetadataReader as part of the call.

[tmat]

@brianrob Step [2] above creates MetadataReaderProvider instance that holds on an ImmutableArray<byte> with the PDB data. At this point the memory is not pinned. Step [3] happens when System.Diagnostics code calls MetadataReaderProvider.GetMetadataReader. At this point the memory is pinned and returned MetadataReader instance holds on the pointer. The code is here: https://source.dot.net/#System.Diagnostics.StackTrace/System/Diagnostics/StackTraceSymbols.cs,116

Once the provider returns MetadataReader the memory can't be unpinned until we are sure no one is using the reader of any related types that point to the memory. This is up to the user of the reader, the reader can't determine that. The caller calls MetadataReaderProvider.Dispose to free the memory.