dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.44k stars 4.76k forks source link

Microsoft.Extensions.Http is too heavy-weight for mobile usages #66863

Open eerhardt opened 2 years ago

eerhardt commented 2 years ago

Description

In profiling the .NET Podcasts application startup time on an Android Pixel 5a, I'm seeing its usage of a named HttpClient using builder.Services.AddHttpClient<ShowsService>(client => {}) as taking up 5% of the startup cost of the app.

Configuration

Regression?

No

Data

Using the startup profiling script at https://github.com/jonathanpeppers/maui-profiling#faq, I'm getting the following results:

(using a full AOT of the app, so to eliminate any JIT time)

Startup time with AddHttpClient

Average(ms): 838.2 Average(ms): 832.4 Average(ms): 835.2

Startup time with just creating a shared HttpClient without DI

Average(ms): 795.5 Average(ms): 799.8 Average(ms): 797.9

Analysis

dotnet-podcasts.speedscope.zip

  1. Looking through the attached speedscope, a large chunk of time is spent creating the named Http client service in AddTransientHelper:

image

A big chunk of that time is due to calling ActivatorUtilities.CreateFactory, which uses System.Linq.Expressions and IL Ref.Emit.

  1. The Logging in Microsoft.Extensions.Http also seems to be adding a decent amount of overhead:

image

Discussion

I wonder if using Microsoft.Extensions.Http on a mobile application is recommended at all. Or if a mobile app should just be following the guidance we give in our docs:

HttpClient is intended to be instantiated once and re-used throughout the life of an application.

@davidfowl @jonathanpeppers @dotnet/ncl

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
### Description In profiling the [.NET Podcasts application](https://github.com/microsoft/dotnet-podcasts) startup time on an Android Pixel 5a, I'm seeing its [usage](https://github.com/microsoft/dotnet-podcasts/blob/392453be5fa55c47ff84b98731d9b71fa6dc489c/src/Mobile/Services/ServicesExtensions.cs#L11-L14) of a named HttpClient using ` builder.Services.AddHttpClient(client => {})` as taking up 5% of the startup cost of the app. ### Configuration * Which version of .NET is the code running on? `6.0` * What OS version, and what distro if applicable? `Android 12` * What is the architecture (x64, x86, ARM, ARM64)? `ARM64` * If relevant, what are the specs of the machine? `Google Pixel 5a` ### Regression? No ### Data Using the startup profiling script at https://github.com/jonathanpeppers/maui-profiling#faq, I'm getting the following results: (using a full AOT of the app, so to eliminate any JIT time) #### Startup time with AddHttpClient Average(ms): 838.2 Average(ms): 832.4 Average(ms): 835.2 #### Startup time with just creating a shared HttpClient without DI Average(ms): 795.5 Average(ms): 799.8 Average(ms): 797.9 ### Analysis [dotnet-podcasts.speedscope.zip](https://github.com/dotnet/runtime/files/8307769/dotnet-podcasts.speedscope.zip) 1. Looking through the attached speedscope, a large chunk of time is spent creating the named Http client service in [AddTransientHelper](https://github.com/dotnet/runtime/blob/0b12d37843e7165fb4c8b794186f19ef43af6c73/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs#L260): ![image](https://user-images.githubusercontent.com/8291187/159086734-4f7f70cd-ef20-404b-b077-ec8f4d2e04e5.png) A big chunk of that time is due to calling [ActivatorUtilities.CreateFactory](https://github.com/dotnet/runtime/blob/0b12d37843e7165fb4c8b794186f19ef43af6c73/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs#L91), which uses System.Linq.Expressions and IL Ref.Emit. 2. The Logging in Microsoft.Extensions.Http also seems to be adding a decent amount of overhead: ![image](https://user-images.githubusercontent.com/8291187/159087742-37a4297e-4b14-4f5b-bb21-318acaf77602.png) ### Discussion I wonder if using Microsoft.Extensions.Http on a mobile application is recommended at all. Or if a mobile app should just be following the guidance we give in [our docs](https://docs.microsoft.com/dotnet/api/system.net.http.httpclient): > HttpClient is intended to be instantiated once and re-used throughout the life of an application. @davidfowl @jonathanpeppers @dotnet/ncl
Author: eerhardt
Assignees: -
Labels: `tenet-performance`, `area-Extensions-HttpClientFactory`
Milestone: -
jonathanpeppers commented 2 years ago

For the podcast app, should we maybe go with something like:

services.TryAddSingleton<HttpClient>(sp => new HttpClient());

And use 1 instance of HttpClient for the entire app?

eerhardt commented 2 years ago

That's similar to what I did: https://github.com/eerhardt/dotnet-podcasts/commit/e0849b2f9444cfc09bab6d3b62ecf2b637946f18

I just didn't use DI to inject an HttpClient. Instead, I just made a static variable in ShowsService.

One issue with injecting an HttpClient is if you are connecting to multiple endpoints. It is easier to set the "BaseAddress", or headers, on different instances of HttpClient.

CarnaViire commented 2 years ago

Yes, I would say it's quite a heavy overhead for the mobile usages - which might not be needed at all. The factory means to solve two problems - socket exhaustion (that can be solved by a static/singleton HttpClient) and not losing DNS changes. I am not sure whether the second problem even exists for mobile usages? Given it's not SocketsHttpHandler that is used there.

ghost commented 2 years ago

This issue has been marked needs-author-action and may be missing some important information.

jonathanpeppers commented 2 years ago

Socket exhaustion definitely seems like something server scenarios would run into. If mobile app is making enough web requests to hit that problem, I would probably recommend "don't do that". Even if a mobile app was downloading lots of files, they should probably limit to 4 (or some number) of concurrent connections

davidfowl commented 2 years ago

The IHttpClientFactory isn't just about socket exhaustion and dns changes, it's about a DI friendly way to configure and pass around the http client. It just so happens that startup time is affected because of IL emit. I'm pretty sure we could come up with something more mobile friendly configuration wise, but I haven't thought through how to work around/fix the startup performance problems.

eerhardt commented 2 years ago

It just so happens that startup time is affected because of IL emit

The Logging in Microsoft.Extensions.Http also seems to be adding a decent amount of overhead

davidfowl commented 2 years ago

Hmmm, maybe worth making that opt-in.