Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allocation details for AspNetCore instrumentation benchmarks #4452

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Apr 27, 2023

Towards #4376

Changes

  • Added allocation details for AspNetCore instrumentation benchmarks for better performance assessment and identifying scope of improvement

Note: This is for .NET 7.

Analysis

I used the .NET Object Allocation Tracking tool: https://learn.microsoft.com/en-us/visualstudio/profiling/dotnet-alloc-tool?view=vs-2022

I used the following setup to compare the allocations of the different scenarios:

var builder = WebApplication.CreateBuilder();
builder.Logging.ClearProviders();
var app = builder.Build();
app.MapGet("/", async context => await context.Response.WriteAsync($"Hello World!"));
_ = app.RunAsync();

var httpClient = new HttpClient();

using var tracerProvider = Sdk.CreateTracerProviderBuilder()
    .AddAspNetCoreInstrumentation()
    .Build();

using var meterProvider = Sdk.CreateMeterProviderBuilder()
   .AddAspNetCoreInstrumentation()
   .Build();

for (int i = 0; i < 100000; i++)
{
    var httpResponse = await httpClient.GetAsync("http://localhost:5000").ConfigureAwait(false);
    httpResponse.EnsureSuccessStatusCode();
}
  1. I ran the code to be monitored in a for loop to easily distinguish between one-time allocations vs hot-path allocations.
  2. I then compared the hot-path allocations for the baseline scenarios and the other scenarios to find out which part of our code caused the allocations (The allocation tool has useful information about call tree, modules, allocation size, etc. which helps with this).

@utpilla utpilla requested a review from a team April 27, 2023 23:49
@utpilla utpilla changed the title Update allocation details for AspNetCore instrumentation benchmarks Add allocation details for AspNetCore instrumentation benchmarks Apr 27, 2023
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #4452 (3dd2670) into main (1b9290e) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4452   +/-   ##
=======================================
  Coverage   84.09%   84.09%           
=======================================
  Files         311      311           
  Lines       12329    12329           
=======================================
  Hits        10368    10368           
  Misses       1961     1961           

see 5 files with indirect coverage changes

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!
Would be good to describe the process you followed to break this down, so it'll be good learning for others too, and can be applied elsewhere.

@cijothomas cijothomas merged commit bb16c20 into open-telemetry:main Apr 28, 2023
@utpilla utpilla deleted the utpilla/Add-allocation-details-for-AspNetCoreBenchmarks branch November 3, 2023 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants