Skip to content

Add a benchmark for aspnetcore#879

Merged
kevingosse merged 5 commits intomasterfrom
kevin/aspnetcorebenchmarks
Sep 2, 2020
Merged

Add a benchmark for aspnetcore#879
kevingosse merged 5 commits intomasterfrom
kevin/aspnetcorebenchmarks

Conversation

@kevingosse
Copy link
Copy Markdown
Contributor

The benchmark goes through a typical customer scenario: receive a request through aspnetcore, and query an external HTTP service.

@kevingosse kevingosse requested a review from a team as a code owner August 25, 2020 14:43
@kevingosse kevingosse added the area:benchmarks Benchmarks, throughput tests, Crank, Bombardier, etc label Aug 25, 2020
@kevingosse
Copy link
Copy Markdown
Contributor Author

/azp run Benchmarks

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@kevingosse kevingosse force-pushed the kevin/aspnetcorebenchmarks branch from aac9ff2 to 308ee2b Compare September 1, 2020 12:42
Comment thread benchmarks/Benchmarks.Trace/Benchmarks.Trace.csproj Outdated
Comment thread benchmarks/Benchmarks.Trace/AspNetCoreBenchmark.cs Outdated

static AspNetCoreBenchmark()
{
Environment.SetEnvironmentVariable(ConfigurationKeys.TraceEnabled, "0");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think you can set this in the TracerSettings below with TraceEnabled = false so you don't have to modify the environment variables

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem is subtle:

to initialize the tracer, I do:

Tracer.Instance = new Tracer(settings, new DummyAgentWriter(), null, null, null);

By calling new Tracer, the static constructor is invoked, which initializes a new tracer before I have time to construct mine. That tracer is fully functional, with background threads and stuff, and adds noise to the benchmark. Adding environment variables is the only way I found to limit what that tracer does in background.
Now that I think about it, I should probably just add a global internal setting that prevents that tracer from being instantiated. I'll look into that in a separate PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we move the creation of the first Tracer instance from the static constructor to the Tracer.Instance getter? If we can do this, it should allow users (including ourselves here) to set Tracer.Instance without creating a not-so-throwaway Tracer instance. Just something to think about, not for this PR.

Copy link
Copy Markdown
Contributor

@zacharycmontoya zacharycmontoya left a comment

Choose a reason for hiding this comment

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

Looks good! Looking forward to having more benchmarks!

@kevingosse kevingosse force-pushed the kevin/aspnetcorebenchmarks branch from 308ee2b to 8143584 Compare September 2, 2020 09:10
@kevingosse kevingosse merged commit 3517c77 into master Sep 2, 2020
@kevingosse kevingosse deleted the kevin/aspnetcorebenchmarks branch September 2, 2020 09:24
kevingosse added a commit that referenced this pull request Sep 2, 2020
* Add benchmark for aspnetcore
@lucaspimentel lucaspimentel added this to the 1.19.3 milestone Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:benchmarks Benchmarks, throughput tests, Crank, Bombardier, etc type:new-feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants