Performance - Consider static reference and pooling per service endpoint of HttpClient

[baywet]

The subject has been out there in the .net community for a couple of years now but the general recommendation regarding HttpClient is not to instanciate and dispose the object for each http call. This is particularly true when building a "service- type" application and/or talking to the same service endpoint for subsequent calls. This is related to the way HttpClient is implemented and opens/keeps alive ports to a single endpoint. Because there's a "lag" between the disposal of a HttpClient and the actual disposal of the underlaying network port, those kind of patterns can lead in a port exhaustion. Also disposing the port every-time induces a memory and CPU overhead. https://stackoverflow.com/questions/22560971/what-is-the-overhead-of-creating-a-new-httpclient-per-call-in-a-webapi-client

I think the following changes should be implemented:

• GetClient method of the HttpService should rely on a static pool (probably implemented with a thread safe dictionary, key being the Uri's authority)
• using's should be removed
• app domain terminating events should be implemented to dispose the dictionary's content

[nhadro]

Along with this, does calling "AquireAADToken" on every request cause a performance issue? The token typically does not expire for between 1 and 8 hours, so does the call to "authContext.AcquireTokenAsync" handle that or should the SDK? This solution saved me a ton of time so thank you as well!

[baywet]

@nhadro I think the SDK was built this way to be "Authentication stack agnostic" and support multiple infrastructure cases (on prem vs online) If you're relying on MSAL it's ok because it has some caching and checking implemented, if you implemented the calls manually to acquire the token, it might be another story indeed. But I don't want to side track too much here, the intent of this issue is to discuss the performance implications in HttpService of the way HttpClient is used

[nhadro]

Good point, yep the HttpClient approach is a good question.

[kenakamu]

first of all, ADAL is fine as it gives back the token if not expired and cached without actually calling the service. For HttpClient, I agree with this if this is not only sample but for production.

How do we want to implement this though? We can simply apply singleton model or use package such as AutoFac. But we also need to think about when we need to recreate it due to its status?

[baywet]

From what I was reading on the internet it's better to have a HttpClient per destination service/ip/fqdn from a network port management perspective. I'd see a solution where we leverage something like a dictionary or so depending on the target FQDN as the key and the HttpClient as the value. That dictionary would be a singleton and internal to this SDK. Also the add/remove operations would be done with locks to ensure thread safety or maybe we should use this guy instead https://msdn.microsoft.com/en-us/library/dd287191(v=vs.110).aspx And yes, the last thing would be to check if the token is still valid for the request to re-challenge auth if needed but I think it'd be easy tweaking the HttpService kind in the same way I implemented ExecuteHttpCallAndRetry What do you think?

[kenakamu]

I will go ahead to implement singleton model so please go ahead to test it.

[baywet]

I'm still doing the testing on my end and so far it's looking good but I think we can close the issue. I'll try to update with the performances improvements results on my project when I have those. Thanks for implementing.

[baywet]

Also documented the authentication and performance consideration in the Wiki https://github.com/kenakamu/UCWA2.0-CS/wiki/Performance-Considerations

[baywet]

Our 9970 unit tests went from 16 minutes on average to 14 minutes, nice improvement!