Reduce IsAzureSynapseOnDemandEndpoint allocations

[vonzshik]

Every time you create a new instance of SqlConnection, if the connection string's RetryCount is equal to 1 (which is the default value) SqlClient will attempt to subtly replace it with 5 if it determines that the host is azure synapse on demand endpoint.

SqlClient/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs

Lines 423 to 429 in 6d2473d

	_connectRetryCount = connString.ConnectRetryCount;
	// For Azure Synapse ondemand connections, set _connectRetryCount to 5 instead of 1 to greatly improve recovery
	// success rate. Note: Synapse should be detected first as it could be detected as a regular Azure SQL DB endpoint.
	if (_connectRetryCount == 1 && ADP.IsAzureSynapseOnDemandEndpoint(connString.DataSource))
	{
	_connectRetryCount = 5;
	}

The problem is that to determine that SqlClient concats prefix -ondemand with a collection of specific endpoints and then checks whether the host contains any endpoint. With 5 endpoints, that's 5 string allocations per each instance of SqlConnection, which in a simple benchmark results in about 40% of total allocations.

SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs

Lines 821 to 832 in 6d2473d

	// check if servername ends with any endpoints
	for (int index = 0; index < s_azureSqlServerEndpoints.Length; index++)
	{
	string endpoint = string.IsNullOrEmpty(prefix) ? s_azureSqlServerEndpoints[index] : prefix + s_azureSqlServerEndpoints[index];
	if (length > endpoint.Length)
	{
	if (string.Compare(dataSource, length - endpoint.Length, endpoint, 0, endpoint.Length, StringComparison.OrdinalIgnoreCase) == 0)
	{
	return true;
	}
	}
	}

Maybe it's better to instantiate them once just like it's done for azure sql server endpoints?

[vonzshik]

@ErikEJ more like making a copy of s_azureSqlServerEndpoints with each endpoint having ONDEMAND_PREFIX prepended and passing a specific array to IsEndpoint instead of allocating a string per each.

SqlClient-azure-synapse-check.patch

[ErikEJ]

@vonzshik Got it!

[ErikEJ]

@vonzshik Planning to do a PR?

[vonzshik]

Not really, so go on ahead.

[ErikEJ]

@vonzshik can you share your benchmark code? I would like to verify the fix from the latest main build

[vonzshik]

// Warmup
var conns = new List<SqlConnection>();
for (var i = 0; i < 100; i++)
{
    var conn = new SqlConnection(ConnectionString);
    await conn.OpenAsync();
    conns.Add(conn);
}

foreach (var conn in conns)
{
    await conn.DisposeAsync();
}

Console.WriteLine("Ready. Press any key to continue...");
Console.ReadKey();

var tasks = new List<Task>(100);

for (var i = 0; i < 1000; i++)
{
    tasks.Clear();

    for (var j = 0; j < 100; j++)
    {
        tasks.Add(Task.Run(async () =>
        {
            await using var conn = new SqlConnection(ConnectionString);
            await conn.OpenAsync();
        }));
    }

    await Task.WhenAll(tasks);
}

Console.WriteLine("Done. Press any key to exit...");
Console.ReadKey();

But from what I remember, it's really just enough to create a new instance of SqlConnection with ConnectionString, so most of the stuff here should be overkill.

[ErikEJ]

@vonzshik Thanks, I was able to confirm no allocations from the method call in the latest build. Now it is this that has allocations.

private static DbConnectionPoolIdentity GetCurrentNative()
{
    DbConnectionPoolIdentity current;
    using (WindowsIdentity identity = WindowsIdentity.GetCurrent())
    {