Skip to content

Commit c39dc6f

Browse files
samsharma2700samsharma2700
committed
Revert "Improve async string perf and fix reading chars with initial offset. (#3377)"
This reverts commit 05df554.
1 parent 7a25a8c commit c39dc6f

File tree

4 files changed

+21
-121
lines changed

4 files changed

+21
-121
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13228,17 +13228,17 @@ bool writeDataSizeToSnapshot
1322813228
||
1322913229
(buff.Length >= offst + len)
1323013230
||
13231-
(buff.Length >= (startOffsetByteCount >> 1) + 1),
13231+
(buff.Length == (startOffsetByteCount >> 1) + 1),
1323213232
"Invalid length sent to ReadPlpUnicodeChars()!"
1323313233
);
1323413234
charsLeft = len;
1323513235

13236-
// If total data length is known up front from the plp header by being not SQL_PLP_UNKNOWNLEN
13237-
// and the number of chars required is less than int.max/2 allocate the entire buffer now to avoid
13238-
// later needing to repeatedly allocate new target buffers and copy data as we discover new data
13239-
if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && stateObj._longlen < (int.MaxValue >> 1))
13236+
// If total length is known up front, the length isn't specified as unknown
13237+
// and the caller doesn't pass int.max/2 indicating that it doesn't know the length
13238+
// allocate the whole buffer in one shot instead of realloc'ing and copying over each time
13239+
if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && len < (int.MaxValue >> 1))
1324013240
{
13241-
if (supportRentedBuff && stateObj._longlen < 1073741824) // 1 Gib
13241+
if (supportRentedBuff && len < 1073741824) // 1 Gib
1324213242
{
1324313243
buff = ArrayPool<char>.Shared.Rent((int)Math.Min((int)stateObj._longlen, len));
1324413244
rentedBuff = true;
@@ -13273,7 +13273,8 @@ bool writeDataSizeToSnapshot
1327313273

1327413274
totalCharsRead = (startOffsetByteCount >> 1);
1327513275
charsLeft -= totalCharsRead;
13276-
offst += totalCharsRead;
13276+
offst = totalCharsRead;
13277+
1327713278

1327813279
while (charsLeft > 0)
1327913280
{
@@ -13291,10 +13292,7 @@ bool writeDataSizeToSnapshot
1329113292
}
1329213293
else
1329313294
{
13294-
// grow by an arbitrary number of packets to avoid needing to reallocate
13295-
// the newbuf on each loop iteration of long packet sequences which causes
13296-
// a performance problem as we spend large amounts of time copying and in gc
13297-
newbuf = new char[offst + charsRead + (stateObj.GetPacketSize() * 8)];
13295+
newbuf = new char[offst + charsRead];
1329813296
rentedBuff = false;
1329913297
}
1330013298

‎src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13395,7 +13395,7 @@ bool writeDataSizeToSnapshot
1339513395
{
1339613396
int charsRead = 0;
1339713397
int charsLeft = 0;
13398-
13398+
1339913399
if (stateObj._longlen == 0)
1340013400
{
1340113401
Debug.Assert(stateObj._longlenleft == 0);
@@ -13405,21 +13405,21 @@ bool writeDataSizeToSnapshot
1340513405

1340613406
Debug.Assert((ulong)stateObj._longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request");
1340713407
Debug.Assert(
13408-
(buff == null && offst == 0)
13409-
||
13408+
(buff == null && offst == 0)
13409+
||
1341013410
(buff.Length >= offst + len)
1341113411
||
13412-
(buff.Length >= (startOffsetByteCount >> 1) + 1),
13412+
(buff.Length == (startOffsetByteCount >> 1) + 1),
1341313413
"Invalid length sent to ReadPlpUnicodeChars()!"
1341413414
);
1341513415
charsLeft = len;
1341613416

1341713417
// If total length is known up front, the length isn't specified as unknown
1341813418
// and the caller doesn't pass int.max/2 indicating that it doesn't know the length
1341913419
// allocate the whole buffer in one shot instead of realloc'ing and copying over each time
13420-
if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && stateObj._longlen < (int.MaxValue >> 1))
13420+
if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && len < (int.MaxValue >> 1))
1342113421
{
13422-
if (supportRentedBuff && stateObj._longlen < 1073741824) // 1 Gib
13422+
if (supportRentedBuff && len < 1073741824) // 1 Gib
1342313423
{
1342413424
buff = ArrayPool<char>.Shared.Rent((int)Math.Min((int)stateObj._longlen, len));
1342513425
rentedBuff = true;
@@ -13454,9 +13454,9 @@ bool writeDataSizeToSnapshot
1345413454

1345513455
totalCharsRead = (startOffsetByteCount >> 1);
1345613456
charsLeft -= totalCharsRead;
13457-
offst += totalCharsRead;
13458-
13459-
13457+
offst = totalCharsRead;
13458+
13459+
1346013460
while (charsLeft > 0)
1346113461
{
1346213462
if (!partialReadInProgress)
@@ -13473,10 +13473,7 @@ bool writeDataSizeToSnapshot
1347313473
}
1347413474
else
1347513475
{
13476-
// grow by an arbitrary number of packets to avoid needing to reallocate
13477-
// the newbuf on each loop iteration of long packet sequences which causes
13478-
// a performance problem as we spend large amounts of time copying and in gc
13479-
newbuf = new char[offst + charsRead + (stateObj.GetPacketSize() * 8)];
13476+
newbuf = new char[offst + charsRead];
1348013477
rentedBuff = false;
1348113478
}
1348213479

@@ -13516,7 +13513,7 @@ bool writeDataSizeToSnapshot
1351613513
&& (charsLeft > 0)
1351713514
)
1351813515
{
13519-
byte b1 = 0;
13516+
byte b1 = 0;
1352013517
byte b2 = 0;
1352113518
if (partialReadInProgress)
1352213519
{

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,11 +1412,6 @@ internal bool SetPacketSize(int size)
14121412
return false;
14131413
}
14141414

1415-
internal int GetPacketSize()
1416-
{
1417-
return _inBuff.Length;
1418-
}
1419-
14201415
///////////////////////////////////////
14211416
// Buffer read methods - data values //
14221417
///////////////////////////////////////
@@ -4088,7 +4083,6 @@ internal void CheckStack(string trace)
40884083
Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack == trace, "The stack trace on subsequent replays should be the same");
40894084
}
40904085
}
4091-
40924086
#endif
40934087
public bool ContinueEnabled => !LocalAppContextSwitches.UseCompatibilityAsyncBehaviour;
40944088

‎src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs‎

Lines changed: 1 addition & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6-
using System.Buffers;
76
using System.Collections.Generic;
87
using System.Data;
98
using System.Data.SqlTypes;
@@ -263,7 +262,7 @@ first_name varchar(100) null,
263262
}
264263

265264
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
266-
public static void CheckNullRowVersionIsDBNull()
265+
public static void CheckNullRowVersionIsBDNull()
267266
{
268267
lock (s_rowVersionLock)
269268
{
@@ -702,94 +701,6 @@ DROP TABLE IF EXISTS [{tableName}]
702701
}
703702
}
704703

705-
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
706-
public static async Task CanGetCharsSequentially()
707-
{
708-
const CommandBehavior commandBehavior = CommandBehavior.SequentialAccess | CommandBehavior.SingleResult;
709-
const int length = 32000;
710-
const string sqlCharWithArg = "SELECT CONVERT(BIGINT, 1) AS [Id], CONVERT(NVARCHAR(MAX), @input) AS [Value];";
711-
712-
using (var sqlConnection = new SqlConnection(DataTestUtility.TCPConnectionString))
713-
{
714-
await sqlConnection.OpenAsync();
715-
716-
StringBuilder inputBuilder = new StringBuilder(length);
717-
Random random = new Random();
718-
for (int i = 0; i < length; i++)
719-
{
720-
inputBuilder.Append((char)random.Next(0x30, 0x5A));
721-
}
722-
string input = inputBuilder.ToString();
723-
724-
using (var sqlCommand = new SqlCommand())
725-
{
726-
sqlCommand.Connection = sqlConnection;
727-
sqlCommand.CommandTimeout = 0;
728-
sqlCommand.CommandText = sqlCharWithArg;
729-
sqlCommand.Parameters.Add(new SqlParameter("@input", SqlDbType.NVarChar, -1) { Value = input });
730-
731-
using (var sqlReader = await sqlCommand.ExecuteReaderAsync(commandBehavior))
732-
{
733-
if (await sqlReader.ReadAsync())
734-
{
735-
long id = sqlReader.GetInt64(0);
736-
if (id != 1)
737-
{
738-
Assert.Fail("Id not 1");
739-
}
740-
741-
var sliced = GetPooledChars(sqlReader, 1, input);
742-
if (!sliced.SequenceEqual(input.ToCharArray()))
743-
{
744-
Assert.Fail("sliced != input");
745-
}
746-
}
747-
}
748-
}
749-
}
750-
751-
static char[] GetPooledChars(SqlDataReader sqlDataReader, int ordinal, string input)
752-
{
753-
var buffer = ArrayPool<char>.Shared.Rent(8192);
754-
int offset = 0;
755-
while (true)
756-
{
757-
int read = (int)sqlDataReader.GetChars(ordinal, offset, buffer, offset, buffer.Length - offset);
758-
if (read == 0)
759-
{
760-
break;
761-
}
762-
763-
ReadOnlySpan<char> fetched = buffer.AsSpan(offset, read);
764-
ReadOnlySpan<char> origin = input.AsSpan(offset, read);
765-
766-
if (!fetched.Equals(origin, StringComparison.Ordinal))
767-
{
768-
Assert.Fail($"chunk (start:{offset}, for:{read}), is not the same as the input");
769-
}
770-
771-
offset += read;
772-
773-
if (buffer.Length - offset < 128)
774-
{
775-
buffer = Resize(buffer);
776-
}
777-
}
778-
779-
var sliced = buffer.AsSpan(0, offset).ToArray();
780-
ArrayPool<char>.Shared.Return(buffer);
781-
return sliced;
782-
783-
static char[] Resize(char[] buffer)
784-
{
785-
var newBuffer = ArrayPool<char>.Shared.Rent(buffer.Length * 2);
786-
Array.Copy(buffer, newBuffer, buffer.Length);
787-
ArrayPool<char>.Shared.Return(buffer);
788-
return newBuffer;
789-
}
790-
}
791-
}
792-
793704
// Synapse: Cannot find data type 'rowversion'.
794705
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
795706
public static void CheckLegacyNullRowVersionIsEmptyArray()

0 commit comments

Comments
 (0)