nCubed
commented
7 years ago From what I can gather, the general recommendation is to have a single instance of HttpClient shared per endpoint. However, most of the recommendations are when dealing with high-load clients; for example, an MVC Controller or Web API endpoint with hundreds (thousands) of concurrent users resulting in hundreds/thousands of requests per second.
In this case, the HttpClient is specifically targeting themoviedb.org's API which is throttled at 40 requests per 10 seconds; this value seems to be hit and miss where sometimes it allows more, sometimes less. Given that the maximum number of allowed requests to themoviedb.org's API is extremely low, creating an HttpClient per request is the least resistant path forward.
I believe a better long-term approach would be to create a single shared HttpClient wrapped in some type of throttling queue to prevent receiving the error of "request count is over the allowed limit".
I have done performance profiling against TheMovieDbWrapper and the CreateClient() call is barely a blip on the timeline. The bottlenecks are on Newtonsoft's side for DeserializeObject. Here's a high-level breakdown when performing 40 calls to IApiMovieRequest.SearchByTitleAsync() and excludes all time allotted for waiting, i.e., async/await wating:
| Percentage | Root Method |
|---|---|
| 24.8% | Newtonsoft.Json.JsonConvert.DeserializeObject(String, JsonSerializerSettings) |
| 13.9% | System.ComponentModel.Composition.Hosting.ExportProvider.GetExportCore(String) |
| 2.5% | DM.MovieApi.MovieDb.Movies.ApiMovieRequest.SearchByTitleAsync(String, Int32, String) |
| 0.62% | DM.MovieApi.ApiRequest.ApiRequestBase.CreateClient() |
Unless you are seeing a specific bottleneck, I'd like to keep the base class as-is for now.
hrafnl
This function:
public async Task<ApiQueryResponse> QueryAsync( string command, IDictionary<string, string> parameters, Func<string, T> deserializer )
{
using( HttpClient client = CreateClient() )
{
string cmd = CreateCommand( command, parameters );
...
creates a HttpClient each time the function is called.
Is that necessary? Would it not be more efficient to create an instance of the HttpClient in the constructor of ApiRequestBase?