Improve performance of Debugger.NotifyOfCrossThreadDependency under a debugger

[davmason]

Fixes #38736

Currently the filtering for whether or not we send notifications lives on the ICorDebug side, which means for every call when a debugger is attached we do all the work to pause the runtime and send an IPC event. This changes the filtering to happen on the runtime side and only send an IPC event if the debugger wants to receive it.

Running the perf test from here: #38736 (comment) on my development machine I get the following results for a speedup of ~40x.

Before (Debugger)   : 19.7009k op/s
Before (No Debugger): 5879.8592k op/s

After  (Debugger)   : 777.8416k op/s
After  (No Debugger): 6044.5201k op/s

[noahfalk]

Your hashtable definition has some optional features enabled that are unneeded, but otherwise looks great!

[mikelle-rogers]

I have a couple of questions.

[mikelle-rogers]

Why is there an && here instead of an ||?

[davmason]

I have && because the NULL TypeInModule is defined as a NULL m_module pointer and a 0 m_typeDef. If one of them is true but not the other, it means that something has created a TypeInModule with invalid data, which is different than a NULL TypeInModule. We shouldn't ever see one or the other, only both

[mikelle-rogers]

Why did you choose to use this hash function?

[davmason]

For a hash function you want it to be quick to calculate, and also relatively unique to each entry. This is very quick to compute and should be unique most of the time. On 64 bit platforms there is a very low chance that the lower 32 bits of a module could be the same as another module and the typeDef could be repeated.

[noahfalk]

[celebrate] Noah Falk reacted to your message:

…

________________________________ From: David Mason ***@***.***> Sent: Tuesday, May 7, 2024 2:47:40 AM To: dotnet/runtime ***@***.***> Cc: Noah Falk ***@***.***>; Review requested ***@***.***> Subject: Re: [dotnet/runtime] Improve performance of Debugger.NotifyOfCrossThreadDependency under a debugger (PR #101864) Merged #101864<#101864> into main. — Reply to this email directly, view it on GitHub<#101864 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABPULQHOUQY3UOL636FIOADZBA6EZAVCNFSM6AAAAABHGFITLKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSG4ZDENJZGEYTMOA>. You are receiving this because your review was requested.Message ID: ***@***.***>

[gaoyang]

Looking forward to experiencing the results immediately 🎉

[akirayamamoto]

Consider back-porting this fix to DotNet 8 since DotNet 9 is a not a LTS release.

[noahfalk]

We wouldn't generally backport performance improvements (even large ones). Part of that is mitigating risk that we introduce a new bug in the servicing release and part is that time spent backporting changes to past releases is time not spent making other improvements. We can certainly see what other feedback we get on this though - its not a foregone conclusion.

In the meantime for many .NET apps you can successfully roll them forward to a new version of .NET runtime without needing to change code or rebuild. Even if you stay on an LTS version for shipping an app that shouldn't mean you are constrained to only debug using that same runtime version.