diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index c5d32a9a41..501ea39b41 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -13088,17 +13088,17 @@ bool writeDataSizeToSnapshot || (buff.Length >= offst + len) || - (buff.Length == (startOffsetByteCount >> 1) + 1), + (buff.Length >= (startOffsetByteCount >> 1) + 1), "Invalid length sent to ReadPlpUnicodeChars()!" ); charsLeft = len; - // If total length is known up front, the length isn't specified as unknown - // and the caller doesn't pass int.max/2 indicating that it doesn't know the length - // allocate the whole buffer in one shot instead of realloc'ing and copying over each time - if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && len < (int.MaxValue >> 1)) + // If total data length is known up front from the plp header by being not SQL_PLP_UNKNOWNLEN + // and the number of chars required is less than int.max/2 allocate the entire buffer now to avoid + // later needing to repeatedly allocate new target buffers and copy data as we discover new data + if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && stateObj._longlen < (int.MaxValue >> 1)) { - if (supportRentedBuff && len < 1073741824) // 1 Gib + if (supportRentedBuff && stateObj._longlen < 1073741824) // 1 Gib { buff = ArrayPool.Shared.Rent((int)Math.Min((int)stateObj._longlen, len)); rentedBuff = true; @@ -13133,8 +13133,7 @@ bool writeDataSizeToSnapshot totalCharsRead = (startOffsetByteCount >> 1); charsLeft -= totalCharsRead; - offst = totalCharsRead; - + offst += totalCharsRead; while (charsLeft > 0) { @@ -13152,7 +13151,10 @@ bool writeDataSizeToSnapshot } else { - newbuf = new char[offst + charsRead]; + // grow by an arbitrary number of packets to avoid needing to reallocate + // the newbuf on each loop iteration of long packet sequences which causes + // a performance problem as we spend large amounts of time copying and in gc + newbuf = new char[offst + charsRead + (stateObj.GetPacketSize() * 8)]; rentedBuff = false; } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 3e9266f897..80c8cbcf22 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -13267,7 +13267,7 @@ bool writeDataSizeToSnapshot { int charsRead = 0; int charsLeft = 0; - + if (stateObj._longlen == 0) { Debug.Assert(stateObj._longlenleft == 0); @@ -13277,11 +13277,11 @@ bool writeDataSizeToSnapshot Debug.Assert((ulong)stateObj._longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); Debug.Assert( - (buff == null && offst == 0) - || + (buff == null && offst == 0) + || (buff.Length >= offst + len) || - (buff.Length == (startOffsetByteCount >> 1) + 1), + (buff.Length >= (startOffsetByteCount >> 1) + 1), "Invalid length sent to ReadPlpUnicodeChars()!" ); charsLeft = len; @@ -13289,9 +13289,9 @@ bool writeDataSizeToSnapshot // If total length is known up front, the length isn't specified as unknown // and the caller doesn't pass int.max/2 indicating that it doesn't know the length // allocate the whole buffer in one shot instead of realloc'ing and copying over each time - if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && len < (int.MaxValue >> 1)) + if (buff == null && stateObj._longlen != TdsEnums.SQL_PLP_UNKNOWNLEN && stateObj._longlen < (int.MaxValue >> 1)) { - if (supportRentedBuff && len < 1073741824) // 1 Gib + if (supportRentedBuff && stateObj._longlen < 1073741824) // 1 Gib { buff = ArrayPool.Shared.Rent((int)Math.Min((int)stateObj._longlen, len)); rentedBuff = true; @@ -13326,9 +13326,9 @@ bool writeDataSizeToSnapshot totalCharsRead = (startOffsetByteCount >> 1); charsLeft -= totalCharsRead; - offst = totalCharsRead; - - + offst += totalCharsRead; + + while (charsLeft > 0) { if (!partialReadInProgress) @@ -13345,7 +13345,10 @@ bool writeDataSizeToSnapshot } else { - newbuf = new char[offst + charsRead]; + // grow by an arbitrary number of packets to avoid needing to reallocate + // the newbuf on each loop iteration of long packet sequences which causes + // a performance problem as we spend large amounts of time copying and in gc + newbuf = new char[offst + charsRead + (stateObj.GetPacketSize() * 8)]; rentedBuff = false; } @@ -13385,7 +13388,7 @@ bool writeDataSizeToSnapshot && (charsLeft > 0) ) { - byte b1 = 0; + byte b1 = 0; byte b2 = 0; if (partialReadInProgress) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 302aae4a58..4d8646002c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1360,6 +1360,11 @@ internal bool SetPacketSize(int size) return false; } + internal int GetPacketSize() + { + return _inBuff.Length; + } + /////////////////////////////////////// // Buffer read methods - data values // /////////////////////////////////////// @@ -4027,6 +4032,7 @@ internal void CheckStack(string trace) Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack == trace, "The stack trace on subsequent replays should be the same"); } } + #endif public bool ContinueEnabled => !LocalAppContextSwitches.UseCompatibilityAsyncBehaviour; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 9c4baf8050..765c54e4ca 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Buffers; using System.Collections.Generic; using System.Data; using System.Data.SqlTypes; @@ -268,7 +269,7 @@ first_name varchar(100) null, } [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] - public static void CheckNullRowVersionIsBDNull() + public static void CheckNullRowVersionIsDBNull() { lock (s_rowVersionLock) { @@ -659,6 +660,94 @@ DROP TABLE IF EXISTS [{tableName}] } } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CanGetCharsSequentially() + { + const CommandBehavior commandBehavior = CommandBehavior.SequentialAccess | CommandBehavior.SingleResult; + const int length = 32000; + const string sqlCharWithArg = "SELECT CONVERT(BIGINT, 1) AS [Id], CONVERT(NVARCHAR(MAX), @input) AS [Value];"; + + using (var sqlConnection = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + await sqlConnection.OpenAsync(); + + StringBuilder inputBuilder = new StringBuilder(length); + Random random = new Random(); + for (int i = 0; i < length; i++) + { + inputBuilder.Append((char)random.Next(0x30, 0x5A)); + } + string input = inputBuilder.ToString(); + + using (var sqlCommand = new SqlCommand()) + { + sqlCommand.Connection = sqlConnection; + sqlCommand.CommandTimeout = 0; + sqlCommand.CommandText = sqlCharWithArg; + sqlCommand.Parameters.Add(new SqlParameter("@input", SqlDbType.NVarChar, -1) { Value = input }); + + using (var sqlReader = await sqlCommand.ExecuteReaderAsync(commandBehavior)) + { + if (await sqlReader.ReadAsync()) + { + long id = sqlReader.GetInt64(0); + if (id != 1) + { + Assert.Fail("Id not 1"); + } + + var sliced = GetPooledChars(sqlReader, 1, input); + if (!sliced.SequenceEqual(input.ToCharArray())) + { + Assert.Fail("sliced != input"); + } + } + } + } + } + + static char[] GetPooledChars(SqlDataReader sqlDataReader, int ordinal, string input) + { + var buffer = ArrayPool.Shared.Rent(8192); + int offset = 0; + while (true) + { + int read = (int)sqlDataReader.GetChars(ordinal, offset, buffer, offset, buffer.Length - offset); + if (read == 0) + { + break; + } + + ReadOnlySpan fetched = buffer.AsSpan(offset, read); + ReadOnlySpan origin = input.AsSpan(offset, read); + + if (!fetched.Equals(origin, StringComparison.Ordinal)) + { + Assert.Fail($"chunk (start:{offset}, for:{read}), is not the same as the input"); + } + + offset += read; + + if (buffer.Length - offset < 128) + { + buffer = Resize(buffer); + } + } + + var sliced = buffer.AsSpan(0, offset).ToArray(); + ArrayPool.Shared.Return(buffer); + return sliced; + + static char[] Resize(char[] buffer) + { + var newBuffer = ArrayPool.Shared.Rent(buffer.Length * 2); + Array.Copy(buffer, newBuffer, buffer.Length); + ArrayPool.Shared.Return(buffer); + return newBuffer; + } + } + } + // Synapse: Cannot find data type 'rowversion'. [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public static void CheckLegacyNullRowVersionIsEmptyArray()