benaadams / Ben.Demystifier

High performance understanding for stack traces (Make error logs more productive)
Apache License 2.0
2.76k stars 118 forks source link

ValueTaskAwaiter no longer annotated with StackTraceHidden #118

Closed bruno-garcia closed 3 years ago

bruno-garcia commented 3 years ago

Running against .NET 5, the test MixedStack fails as a new frame shows up:

TResult System.Runtime.CompilerServices.ValueTaskAwaiter<TResult>.GetResult().

In this change to the runtime (corefx) you've added the attributes but it seems like it's are gone now.

The current approach is to either rely on the attribute or fallback to the type checks: https://github.com/benaadams/Ben.Demystifier/blob/87375e9013db462ad5af21bc308bc73c63cfe919/src/Ben.Demystifier/EnhancedStackTrace.Frames.cs#L665-L703

I could open a PR that removes the if and tries both strategies. Let me know if this is the approach you want to take, or another?

benaadams commented 3 years ago

Ah; made a change so the runtime also hid stacks marked with MethodImplOptions.AggressiveInlining which is detected with mb.MethodImplementationFlags & MethodImplAttributes.AggressiveInlining; which meant StackTraceHidden wasn't needed for these methods.

https://github.com/dotnet/runtime/blob/7c18d4d6488dab82124d475d1199def01d1d252c/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs#L348-L361

So probably should take the same approach?