typo: hander -> handler #749
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nschonni
requested review from
BillWagner,
JRAlexander and
rpetrusha
as code owners
|
Changes to C# and VB samples look good! @JRAlexander can you confirm we can make the changes to the TSV files? It might contain real user data that we don't wanna change. Please advise. |
JRAlexander
suggested changes
Mar 27, 2019
There was a problem hiding this comment.
Yes, @mairaw, the two *.tsr files are real data that we don't want to change. Please remove them from the PR. Thanks!
| 21371 area-System.Net Faster SslStream SslStream apparently shows up as bottleneck in server-side scenarios. Info from @davidfowl offline: * Benchmarks observed: TechEmpower benchmark - see [results](https://msit.powerbi.com/view?r=eyJrIjoiMzFiNDdjNWEtODRiZS00ZDM3LTg4YWEtMGQ3OWQ2ZTc3MDUyIiwidCI6IjcyZjk4OGJmLTg2ZjEtNDFhZi05MWFiLTJkN2NkMDExZGI0NyIsImMiOjV9) * PlainText (with pipelining): 1.31M RPS * PlainText with Https (with pipelining): 156K RPS ... clear large ~10x overhead at this moment * [HttpsConnectionAdapter code](https://github.com/aspnet/KestrelHttpServer/blob/c83f606b220714fa7b1b6a6a91b16d78cee6005f/src/Microsoft.AspNetCore.Server.Kestrel.Https/HttpsConnectionAdapter.cs) in Kestrel * Idea: Compare SSL slowdown in competing technologies (incl. C) to get the idea where it could be potentially pushed. | ||
| 21373 area-System.Net Fix DefaultProxyCredentials edge case on NETFX After my PR #21325, I discovered an edge case where we need to check for a non-null Proxy property in the underlying WebRequest. On .NET Framework, System.Net.Http is built on WebRequest (HttpWebRequest) and is subject to side-affects from it. The static property WebRequest.DefaultWebProxy affects the initial value of WebRequest.Proxy. So, it's possible that a developer may have set WebRequest.DefaultWebProxy to null prior to using System.Net.Http in the app. The net affect of this is that there won't be any proxy used as the system default proxy and thus the HttpClientHandler.DefaultProxyCredentials is ignored. On .NET Core, there is no relationship between System.Net.Http and System.Net.WebRequest. So, in all cases on .NET Core, where a developer has specified HttpClientHander.UseProxy=true and HttpClientHandler.Proxy = null (default values), the handler will always try to use the system default proxy (if configured). | ||
| 21373 area-System.Net Fix DefaultProxyCredentials edge case on NETFX After my PR #21325, I discovered an edge case where we need to check for a non-null Proxy property in the underlying WebRequest. On .NET Framework, System.Net.Http is built on WebRequest (HttpWebRequest) and is subject to side-affects from it. The static property WebRequest.DefaultWebProxy affects the initial value of WebRequest.Proxy. So, it's possible that a developer may have set WebRequest.DefaultWebProxy to null prior to using System.Net.Http in the app. The net affect of this is that there won't be any proxy used as the system default proxy and thus the HttpClientHandler.DefaultProxyCredentials is ignored. On .NET Core, there is no relationship between System.Net.Http and System.Net.WebRequest. So, in all cases on .NET Core, where a developer has specified HttpClientHandler.UseProxy=true and HttpClientHandler.Proxy = null (default values), the handler will always try to use the system default proxy (if configured). | ||
| 21376 area-System.Runtime Fix failing System.Runtime.Interop.Tests on ILC "Fixes https://github.com/dotnet/corefx/issues/21348 Override all the remaining Reflection apis whose default implementation is ""throw"". This will increase the chance of success in the cases where the two CoreLib's have different dependencies on Reflection objects passed into them. We actually try very hard to avoid these but there are some cases where the cost of doing that is too high - CustomAttributeExtensions being one of them." |
There was a problem hiding this comment.
This is real comment data that we don't want to change. Please remove the .tsv files from the PR. Thanks!
There was a problem hiding this comment.
Rebased out those changes
mairaw
approved these changes
Mar 28, 2019
mairaw
added
the
changes-addressed
JRAlexander
approved these changes
Mar 28, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to dotnet/dotnet-api-docs#2155