Align Task usage / ArrayPool / IsColumnEncryptionSupported netcore/netfx #2982
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2982 +/- ##
==========================================
- Coverage 72.50% 72.46% -0.04%
==========================================
Files 288 288
Lines 59529 59507 -22
==========================================
- Hits 43159 43122 -37
- Misses 16370 16385 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
In general, I think this is a decent change. I'm hesitant to start touching things in the TdsParser, but these changes seem to be pretty constrained. Only a few comments. Not sure about the feasibility of using stackalloc instead of array pool in the last commit.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
Outdated
Show resolved
Hide resolved
| @@ -12807,119 +12807,127 @@ private async Task WriteXmlFeed(XmlDataFeed feed, TdsParserStateObject stateObj, | |||
| private async Task WriteTextFeed(TextDataFeed feed, Encoding encoding, bool needBom, TdsParserStateObject stateObj, int size, bool useReadBlock) | |||
| { | |||
| Debug.Assert(encoding == null || !needBom); | |||
| char[] inBuff = new char[constTextBufferSize]; | |||
| char[] inBuff = ArrayPool<char>.Shared.Rent(constTextBufferSize); | |||
There was a problem hiding this comment.
For something like this, can we just do a stackalloc?
There was a problem hiding this comment.
That's unsafe territory... I don't think I'm the right person for that :)
(This just mirrors the implementation from netcore to netfx)
| byte[] buff = new byte[constBinBufferSize]; | ||
| int nWritten = 0; | ||
| do | ||
| byte[] buff = ArrayPool<byte>.Shared.Rent(constBinBufferSize); |
There was a problem hiding this comment.
Same question here, can we use a stackalloc?
There was a problem hiding this comment.
That's unsafe territory... I don't think I'm the right person for that :)
(This just mirrors the implementation from netcore to netfx)
Part of #2953
Aligning Task return, bringing over ArrayPool usage and aligning IsColumnEncryptionSupported changes between netcore and netfx