Skip to content

Use the new CreateSlimBuilder API #1784

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

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Conversation

eerhardt
Copy link
Contributor

The dotnet new api -aot template is getting updated to use the new slim API in dotnet/aspnetcore#46040. Updating the benchmark to match.

NOTE: this depends on getting a build with dotnet/aspnetcore#46040 merged. So it will fail until then.

The `dotnet new api -aot` template is getting updated to use the new slim API in dotnet/aspnetcore#46040. Updating the benchmark to match.
@eerhardt
Copy link
Contributor Author

FYI - dotnet/aspnetcore#46040 has been merged. I'm not sure how long it takes for a new aspnetcore build to propagate to the benchmarks.

cc @mitchdenny @sebastienros

@mitchdenny
Copy link
Contributor

Do we still need to clear providers with slim builder?

These settings are no longer needed.
@sebastienros
Copy link
Member

Once a PR is merged it should take a just a few hours to get a build available in the public feeds, and picked up by crank. Crank checks the nuget feeds directly.

@eerhardt
Copy link
Contributor Author

Do we still need to clear providers with slim builder?

I tested it a few times locally with and without clear providers. I was seeing 0.5%-1% drop in RPS when I don't clear.

With ClearProviders():

| Requests/sec | 480,957 | |
| Requests | 7,262,492 | |

Without:

| Requests/sec | 478,373 | |
| Requests | 7,223,112 | |

So for now I left it in.

@sebastienros sebastienros merged commit 9ad8e3f into aspnet:main Jan 27, 2023
@sebastienros
Copy link
Member

@eerhardt with and without ClearProviders, are there any changes in the console output, for instance wrt "Application started" message and other "lifetime" messages?

@eerhardt eerhardt deleted the patch-1 branch January 27, 2023 21:28
@eerhardt
Copy link
Contributor Author

eerhardt commented Jan 27, 2023

No, no change in the console output.

I debugged the app, and now with CreateSlimBuilder there are no ILoggerProvider services registered anymore, which is what ClearProviders() removes:

https://github.com/dotnet/runtime/blob/007df054a526ed9e3dc70b43bfa330943bd4816a/src/libraries/Microsoft.Extensions.Logging/src/LoggingBuilderExtensions.cs#L48

So I guess the above numbers were just within margin of error?

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.

3 participants