SqlClient reduce memory use for repeated ExecuteNonQueryAsync

[Wraith2]

When using SqlCommand.ExecuteNonQueryAsync delegates for the async continuations and convertion from BeginEnd to Task are allocated on each call. This PR introduces a new class called ExecuteAsyncHelper which allows lazy creation and caching of those delegates for each connection object. This is of benefit when using the same command object to call ExecuteNonQueryAsync multiple times but has a very minor (a single wrapping object) degradation on single call per command, in practice I expect this additional memory to get lost in the noise and I think I've made enough improvements in other PR's to have a little space to work with.

Continuing the theme of reducing repeated delegate creation I have added some a cached delegate and context objects to SqlReferenceCollection which is used when validating that the connection is in a state allowing execution, this will affect all kinds of query, the general pattern used is one I've found elsewhere in the corefx codebase:

    FindLiveReaderContext context = Interlocked.Exchange(ref cachedFindLiveReaderContext, null) ?? new FindLiveReaderContext();
    context.Setup(command);
    SqlDataReader retval = FindItem(DataReaderTag, context.Func);
    context.Clear();
    Interlocked.CompareExchange(ref cachedFindLiveReaderContext, context, null);

using interlocked to try and re-use a single item per collection if possible and falling back to the previous allocating behaviour in the case where it isn't available. This should allow low contention scenarios to be low allocation and ramp up to meet high contention needs without bottlenecking.

The performance improvements are modest:

Method	Mean	Error	StdDev	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
AsyncFloat32 master	212.9 ms	1.896 ms	1.681 ms	-	-	-	1.95 KB
AsyncFloat32 branch	212.0 ms	1.479 ms	1.235 ms	-	-	-	1.47 KB

This is mostly because the dominating allocations are unavoidable task machinery or waiting another PR SqlParameter cleanup.
.

All the usual functional and manual tests pass.
/cc @afsanehr @tarikulsabbir, @Gary-Zh , @David-Engel

[AfsanehR-zz]

@Wraith2 Thanks! We will start review on this changes.

[Wraith2]

If you're time constrained the changes in #34049 will be more valuable than this PR.

[AfsanehR-zz]

same as #36664 will need to run DataAccess tests.

[Wraith2]

Merge updated. Can you make sur to squash when merging to master please.

[ViktorHofer]

@saurabh500 please review, looks like this is ready.

[ViktorHofer]

/azp run corefx-ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Wraith2]

All SqlClient PR's are waiting on the team integrating Microsoft.Data.SqlClient.

[karelz]

Marking as blocked then.

[stephentoub]

This is marked as 3.0 and "blocked". When is it going to be unblocked/merged or closed? The bar is moving higher for 3.0 changes, and it's unlikely to make it in if it's not merged in the next week or so; and if it is merged in the next week or so, there will be very little time to react to any fallout.

[Wraith2]

Given the speed of movement on SqlClient and with no review so far it's too large a change to consider at this point. I'd advise moving it to future.

I was advised that work done in System.Data.SqlClient would be ported into Microsoft.Data.SqlClient so it may not end up getting merged here. Until we see some code from M.D.SqlClient pretty much everything related to sqlclient is blocked and unknown so the same applies to all my open PR's.

[maryamariyan]

@Wraith2 doesn't seem like this PR is being actively worked on or will be done in the next few days.
Also some files have conflict now.
@divega is there anyone who can do the review while @saurabh500 is away?
If not we could close this PR and reopen later.

[Wraith2]

Unless someone reviews it and requests changes it's done, it doesn't need further work from me. I didn't update it because there was no point constantly rebasing or merging when there was no-one looking at it. Should anyone decide to pick it up and investigate it's a simple matter to get it up to date.

This and all my other SqlClient PR's are just waiting for the SqlClient team.

[maryamariyan]

cc: @afsanehr

[danmoseley]

@cheenamalhotra, @Gary-Zh , @David-Engel what do you want to do with this PR? Perhaps at this point it shoudl transfer to https://github.com/dotnet/SqlClient ?

I am going to close it since it's been 2 months but if this is a valuable change it would be good to make sure it is not lost. Thanks @Wraith2 .

[cheenamalhotra]

Hi @danmosemsft @Wraith2

Yes the PR changes will be pulled up in Microsoft.Data.SqlClient, and they are being tracked. We have not forgotten them, they're just not falling in our priority list as of now. We're clearing up backlog and preparing for GA hence not adding more changes for M.D.SqlClient.

Post GA 1.0, we will come back to them and if needed, we shall consider back-porting to System.Data.SqlClient as well, depending on the changes.

Thank you for your patience! 🙏

[cheenamalhotra]

Also adding acknowledgement note for @Wraith2

This PR is being tracked and will be picked up for dotnet/SqlClient. Appreciate your patience for some more time while we come back to review and merge your contributions! 🙏