Basic infrastructure for binder tracing

[elinor-fung]

Add basic infrastructure for binder tracing

• Helper class for firing start/stop event if tracing is enabled
  • Call into managed activity tracker to start/stop activities (only if tracing is enabled, so perf hit should already be expected from enabling tracing)
• New AssemblyBindStart and AssemblyBindStop events
• Fire events in AppDomain::BindAssemblySpec and AssemblyNative::LoadFromPEImage
• Binder tracing test project and event listener

Notes:

• Properties on the new events AssemblyBindStart and AssemblyBindStop events aren't properly populated yet.
• AssemblyBindStop is not fired if tracing was not enabled at the start of the bind. This will be updated when we actually populate all the events' properties.
• This is completely ignoring binds for System.Private.CoreLib.resources. That load can be triggered by the creation of an EventSource (which can be triggered by ActivityTracker), so calling back into ActivityTracker could result in infinite recursion between the bind and tracing the bind. Potential options:
  • Don't trace binds for System.Private.CoreLib.resources (this PR, no events at all)
  • Add some param to ActivityTracker functions to indicate it should not use any EventSource during those calls
  • Don't start/stop an activity for System.Private.CoreLib.resources binds (events, but without correlation)

[lpereira]

Just a few minor nits, but LGTM otherwise. I'll test it soon to see if I'll hit the same issues I had with the prototype.

[lpereira]

It's passing on Linux (on my machine), so I guess the changes in ActivityTracker won't be needed anymore.

[jkotas]

I know we like to call this assembly binding for historic reasons, but our documentation and customers call this assembly loading. E.g.: https://docs.microsoft.com/en-us/dotnet/core/dependency-loading/loading-managed

I would stick with assembly loading for anything that is public facing.

[elinor-fung]

Does that mean we don't want to call it assembly binding in the events either? I put them under the Binder keyword (that used to be Fusion); I didn't want to have them under Loader, since I think we'd want to enable them separately. The new events are also under the AssemblyBinder task and named AssemblyBind*.

[jkotas]

Would it make sense to call this AssemblyLoader / AssemblyLoad ?

I would be interested to know what other people (e.g. PMs) think about the right public facing names for this.

[elinor-fung]

Or maybe also AssemblyResolver/AssemblyResolve? Since we have the AssemblyLoadContext.Resolving and AppDomain.AssemblyResolve events.

Will check with PMs.

[jkotas]

I am wondering whether AssemblyLoadContext would be a more appropriate place for this. Assembly is a grab bag of everything. AssemblyLoadContext is very specific to assembly loading and presumably quite a bit of the tracing will live on ALC.

Also, this method can be called just IsTracingEnabled.

[jkotas]

LGTM modulo one nit. Thanks!

[filipnavara]

This broke Mono and CoreRT builds. NativeRuntimeEventSource is present in the non-shared partition and thus missing on the other runtimes. To solve it, we should:

• Move NativeRuntimeEventSource.cs to shared partition, guarded by the FeaturePerfTracing condition in .csproj
• Guard the code in ActivityTracker.cs with FEATURE_PERFTRACING.

/cc @marek-safar @jkotas

[jkotas]

I think these assembly loader specific tracing methods should not be in ActivityTracker.cs at all. They should be in some assembly loader specific file (that is not in the shared partition - like the rest of the assembly loader).

[jkotas]

Could you please add this to Mono somewhere to workaround this?

    // TODO: Delete. Temporary workaround to fix build break.
    internal class NativeRuntimeEventSource
    {
        public static System.Diagnostics.Tracing.EventSource Log => default;
    }

[filipnavara]

Yeah, that is also an option and I agree it belongs somewhere else. We may eventually want to adapt this tracing for Mono too and share it but right now I am more concerned about the build breaks.

[filipnavara]

Could you please add this to Mono somewhere to workaround this?

    // TODO: Delete. Temporary workaround to fix build break.
    internal class NativeRuntimeEventSource
    {
        public static System.Diagnostics.Tracing.EventSource Log => default;
    }

Sure, I am on it.

[jkotas]

I have added dotnet/corefx@3eb5f9e to the mirror PR in CoreFX that will propagate around in the next round.

[filipnavara]

Thanks!

[elinor-fung]

Sorry, completely missed the shared/non-shared thing - #27535