You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds infrastructure to the async reader snapshot to allow the last packet of the snapshot to be captured as a continue point for the snapshot. On replay if a continue point is available it is used which skips re-doing the previous work improving speed.
The name and behaviour of the appcontext switch being used will need agreement and work. This continue capability requires the partial packet capability so legacy mode enabled on partial packets will force this feature to be disables. I expect there to be some discussion around the names and exact details so the current PR is in dev form, easy to see and change.
Benchmarks for reading a 20Mib string from a local sql server. As payload sizes increase and latency increases speed differences should be more noticeable. I invite and request people to do some benchmarking to make sure we're aware of the performance characteristics of this change.
BenchmarkDotNet v0.14.0, Windows 11 (10.0.26100.2314)
.NET SDK 9.0.200
[Host] : .NET 8.0.13 (8.0.1325.6609), X64 RyuJIT
DefaultJob : .NET 8.0.13 (8.0.1325.6609), X64 RyuJIT
| Method | UseContinue | Mean | Error | StdDev | Gen0 | Gen1 | Gen2 | Allocated |
|------- |------------ |----------:|----------:|----------:|----------:|----------:|----------:|----------:|
| Async | False | 757.51 ms | 15.053 ms | 36.642 ms | 2000.0000 | 1000.0000 | - | 101.49 MB |
| Sync | False | 39.40 ms | 0.543 ms | 0.508 ms | 2000.0000 | 888.8889 | 777.7778 | 80.14 MB |
| Async | True | 49.45 ms | 0.901 ms | 1.376 ms | 4333.3333 | 3555.5556 | 1111.1111 | 101.51 MB |
| Sync | True | 40.09 ms | 0.476 ms | 0.445 ms | 2000.0000 | 888.8889 | 777.7778 | 80.14 MB |
Commenter does not have sufficient privileges for PR 3161 in repo dotnet/SqlClient
Unfortunately, new security requirements restrict our ability to run CI for non-maintainers. Feel free to ping the team alias (@dotnet/sqlclientdevteam) or any of us directly and we'll do our best to trigger a pipeline run ASAP.
Based on the feedback provided I have added a new compatibility switch called Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour which will disable the new behaviour. Is CompatProcessSni enabled the new switch will automatically return true. The new async behaviour is now enabled by default in this PR. Can someone re-run the CI please.
The jobs are failing because the manual tests are timing out. The tests are timing out because stream operations are hitting a path that I wasn't aware of (and didn't break the last two times I reimplemented this) so more work is needed.
I've made many changes to get streams working. Most things seem to work now but it's hard to tell with how test experience is locally. Can someone on the @dotnet/sqlclientdevteam run the CI please.
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
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.
paulmedynski
left a comment
Split out from #2608 per discussion detailed in #2608 (comment)
Adds infrastructure to the async reader snapshot to allow the last packet of the snapshot to be captured as a continue point for the snapshot. On replay if a continue point is available it is used which skips re-doing the previous work improving speed.
The name and behaviour of the appcontext switch being used will need agreement and work. This continue capability requires the partial packet capability so legacy mode enabled on partial packets will force this feature to be disables. I expect there to be some discussion around the names and exact details so the current PR is in dev form, easy to see and change.
Benchmarks for reading a 20Mib string from a local sql server. As payload sizes increase and latency increases speed differences should be more noticeable. I invite and request people to do some benchmarking to make sure we're aware of the performance characteristics of this change.
/cc @MichelZ and @ErikEJ for interest