Regression and Perf Fixes

[tarekgh]

This change is fixing two issues:

• Fixes a regression of setting the recording trace flags on the Activity when having a parent context and the samplers returning a recording flag on. We used to use the parent context trace flags but now we consider the sampler returned value.
• Fixes extra allocation which was happening when recording a measurement in the metrics instruments. Teh original written code was working fine on .NET 5.0 and 6.0 as the jit was avoiding the extra allocation. But on .NET Core and Full Framework this optimization was not happening. The change here is to avoid this extra allocation.

[ghost]

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This change is fixing two issues:

• Fixes a regression of setting the recording trace flags on the Activity when having a parent context and the samplers returning a recording flag on. We used to use the parent context trace flags but now we consider the sampler returned value.
• Fixes extra allocation which was happening when recording a measurement in the metrics instruments. Teh original written code was working fine on .NET 5.0 and 6.0 as the jit was avoiding the extra allocation. But on .NET Core and Full Framework this optimization was not happening. The change here is to avoid this extra allocation.

Author:	tarekgh
Assignees:	-
Labels:	area-System.Diagnostics.Tracing
Milestone:	-

[tarekgh]

CC @reyang @cijothomas @victlu

Thanks to @victlu for catching these issues that early :-)

[noahfalk]

LGTM!

[tarekgh]

#52685

[tarekgh]

The failure in the CI leg runtime (Libraries Test Run release mono Linux x64 Debug) is unrelated to the change and tracked by the issue #44689

[tarekgh]

/backport to release/6.0-preview5

[github-actions]

Started backporting to release/6.0-preview5: https://github.com/dotnet/runtime/actions/runs/857818026

[CodeBlanch]

@tarekgh Hey I was just noticing this change. Would you indulge me on a couple of questions?

• Why remove the else if from the checks?

• I understand the switch from is to typeof(T) is because some frameworks allocate on is. If we added back the old version under a preprocessor directive for >= NET5.0 would it then be allocation free for users on NET5.0 or NET6.0? Right now those users will see at least one boxing from the object cast, no?

[tarekgh]

Why remove the else if from the checks?

It doesn't matter as the JIT optimize the code anyway and will remove all other code when generating type specific code.

I understand the switch from is to typeof(T) is because some frameworks allocate on is. If we added back the old version under a preprocessor directive for >= NET5.0 would it then be allocation free for users on NET5.0 or NET6.0? Right now those users will see at least one boxing from the object cast, no?

No, boxing will be optimized away too. no boxing will occur. try it yourself writing some simple code like:

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        internal static int NotifyMeasurement<T>(T measurement) where T : struct
        {
            if (typeof(T) == typeof(int))
            {
                return (int)(object)measurement;
            }
            if (typeof(T) == typeof(long))
            {
                return (int)(long)(object)measurement;
            }
            return 0;
        }

        [Benchmark]
        public void TestAllocation()
        {
            int t = 0;
            for (int i = 0; i < 10; i++)
            {
                t += NotifyMeasurement<int>(i);
                t += NotifyMeasurement<long>(i);
            }
        }

and you will see no allocation happening

|         Method |     Mean |     Error |    StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------- |---------:|----------:|----------:|------:|------:|------:|----------:|
| TestAllocation | 6.096 ns | 0.0659 ns | 0.0584 ns |     - |     - |     - |         - |

[CodeBlanch]

Good stuff thanks @tarekgh! Is this JIT magic documented anywhere? A cheat sheet of things it will optimize away would be great to have.