Port Performance Improvements from dotnet/corefx#35363 and dotnet/corefx#40732

[cheenamalhotra]

This PR ports changes from dotnet/corefx#35363 and dotnet/corefx#40732 PRs to bring performance improvements in Microsoft.Data.SqlClient.

• Tested for SSL and non-SSL connections on multiple client environments.

[Wraith2]

LGTM.

Not specific to this PR but given the problem the original introduced I wondered it if was worth adding some new tests which just read a single table contents completely from northwind in different configurations of connection strings, e.g. with/without encryption, mars, win/unix. We'd have to use private reflection to force a TDS implementation unless the decision to allow the environment flag in release is changed. As you know it's time consuming to have to switch os and run multiple complete test sequences to get good coverage and it's leading to holes. What do you think?

[cheenamalhotra]

@Wraith2

We're handling them in CI Builds. Public CI Builds will come soon so you'll be able to see the combinations being run.

We're currently testing matrix configurations for NetCore/NetFx, SQL Server/Azure, Encrypt On/Off, Win/Unix, etc. MARS and other features are part of tests, and are tested in these configurations. We will add more in future as we figure out missing code coverage. There's a long way ahead!

Also as a ground rule, we all contributors must ensure code coverage for new lines of change is as close to 100%, so it doesn't drop existing code coverage, and if a scenario is not covered in all these cases, we'll have to make sure they are added with PRs.

[Wraith2]

Getting the manual tests run in combinations in CI will be very useful. I can see getting things working locally and then using draft PR's to run the entire suite improving testing a great deal over corefx.

[Wraith2]

Something to note from the original thread. There is a pre-existing unix bug that surfaces occasionally as an unexpected timeout.

The error I fixed was in SNITcpHandle.Receive, it returns a packet as an out parameter but if an error occurs that out parameter is not set to null. Changing that method so that errors always return null removed the error I was looking into (which was in a similar test) and the timeout I'd seen before. Might be worth trying.

when I advanced my working copy to enable per-connection packet pools I had to spend a few days tracing individual packets to identify the error and that's where it was.

[Wraith2]

It would be good to get this into 1.2 as well. With the small change I mentioned above to SNITcpHandle.Receive it may improve reliability as well as performance.

[David-Engel]

LGTM