Skip to content

Commit d4a982f

Browse files
authored
Reduce allocations when Activity is enabled (#11020)
- Remove string allocations caused by DiagnosticSource.Stop/StartActivity - Pass the HttpContext directly as the object for StartActivity and StopActivity to avoid the anonymous object allocation. - Though it's a bit ugly, added an HttpContext property to DefaultHttpContext to avoid breaking back-compat (which had to do reflection to get the HttpContext property anyways)
1 parent c89055b commit d4a982f

File tree

3 files changed

+33
-3
lines changed

3 files changed

+33
-3
lines changed

src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ internal class HostingApplicationDiagnostics
1616
private static readonly double TimestampToTicks = TimeSpan.TicksPerSecond / (double)Stopwatch.Frequency;
1717

1818
private const string ActivityName = "Microsoft.AspNetCore.Hosting.HttpRequestIn";
19-
private const string ActivityStartKey = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start";
19+
private const string ActivityStartKey = ActivityName + ".Start";
20+
private const string ActivityStopKey = ActivityName + ".Stop";
2021

2122
private const string DeprecatedDiagnosticsBeginRequestKey = "Microsoft.AspNetCore.Hosting.BeginRequest";
2223
private const string DeprecatedDiagnosticsEndRequestKey = "Microsoft.AspNetCore.Hosting.EndRequest";
@@ -278,7 +279,7 @@ private Activity StartActivity(HttpContext httpContext, out bool hasDiagnosticLi
278279
if (_diagnosticListener.IsEnabled(ActivityStartKey))
279280
{
280281
hasDiagnosticListener = true;
281-
_diagnosticListener.StartActivity(activity, new { HttpContext = httpContext });
282+
StartActivity(activity, httpContext);
282283
}
283284
else
284285
{
@@ -293,12 +294,32 @@ private void StopActivity(HttpContext httpContext, Activity activity, bool hasDi
293294
{
294295
if (hasDiagnosticListener)
295296
{
296-
_diagnosticListener.StopActivity(activity, new { HttpContext = httpContext });
297+
StopActivity(activity, httpContext);
297298
}
298299
else
299300
{
300301
activity.Stop();
301302
}
302303
}
304+
305+
// These are versions of DiagnosticSource.Start/StopActivity that don't allocate strings per call (see https://github.com/dotnet/corefx/issues/37055)
306+
private Activity StartActivity(Activity activity, HttpContext httpContext)
307+
{
308+
activity.Start();
309+
_diagnosticListener.Write(ActivityStartKey, httpContext);
310+
return activity;
311+
}
312+
313+
private void StopActivity(Activity activity, HttpContext httpContext)
314+
{
315+
// Stop sets the end time if it was unset, but we want it set before we issue the write
316+
// so we do it now.
317+
if (activity.Duration == TimeSpan.Zero)
318+
{
319+
activity.SetEndTime(DateTime.UtcNow);
320+
}
321+
_diagnosticListener.Write(ActivityStopKey, httpContext);
322+
activity.Stop(); // Resets Activity.Current (we want this after the Write)
323+
}
303324
}
304325
}

src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public DefaultHttpContext(Microsoft.AspNetCore.Http.Features.IFeatureCollection
2424
public override Microsoft.AspNetCore.Http.ConnectionInfo Connection { get { throw null; } }
2525
public override Microsoft.AspNetCore.Http.Features.IFeatureCollection Features { get { throw null; } }
2626
public Microsoft.AspNetCore.Http.Features.FormOptions FormOptions { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
27+
[System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
28+
public Microsoft.AspNetCore.Http.HttpContext HttpContext { get { throw null; } }
2729
public override System.Collections.Generic.IDictionary<object, object> Items { get { throw null; } set { } }
2830
public override Microsoft.AspNetCore.Http.HttpRequest Request { get { throw null; } }
2931
public override System.Threading.CancellationToken RequestAborted { get { throw null; } set { } }

src/Http/Http/src/DefaultHttpContext.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.ComponentModel;
67
using System.Security.Claims;
78
using System.Threading;
89
using Microsoft.AspNetCore.Http.Features;
@@ -157,6 +158,12 @@ public override ISession Session
157158
}
158159
}
159160

161+
// This property exists because of backwards compatibility.
162+
// We send an anonymous object with an HttpContext property
163+
// via DiagnosticSource in various events throughout the pipeline. Instead
164+
// we just send the HttpContext to avoid extra allocations
165+
[EditorBrowsable(EditorBrowsableState.Never)]
166+
public HttpContext HttpContext => this;
160167

161168
public override void Abort()
162169
{

0 commit comments

Comments
 (0)