Slowness when using large number of dynamic parameters. Fixes #1537

[jehhynes]

Calling command.Parameters.Contains(name) for every parameter is an n-squared algorithm for many database implementations and can cause performance degredation when there is a large number of parameters. This fixes that performance issue.

[mgravell]

I like this, but can I ask you to pull in (or at least get your thoughts on) ddc6737 ? this avoids paying the overhead for small scenarios, which is the majority of cases

(I'm trying not to mess up the commit history by stomping it)

[jehhynes]

I like this, but can I ask you to pull in (or at least get your thoughts on) ddc6737 ? this avoids paying the overhead for small scenarios, which is the majority of cases

(I'm trying not to mess up the commit history by stomping it)

I think your changes are good. You might even set the threshold to 20 instead of 10, but some benchmarking would be necessary to pinpoint the sweet spot where the Dictionary overhead outbalances the performance degradation looking up parameters.

[mgravell]

@jehhynes do you want to cherry-pick it into your branch so that I can do everything in one squashed merge?

[jehhynes]

@mgravell Knowledge is power. I did up a benchmark so we can see what we're dealing with. I think a threshold of 8 or 10 would be ok, but not really necessary. The straightforward implementation of always using the HashSet/Dictionary is good in the single digits as well as higher.

If we really wanted to be anal about it we could compare my HashSet implementation to your Dictionary implementation. From my research, HashSet may actually be a tad faster since it's only storing the keys and not a reference to the parameter.

[mgravell]

without the dictionary, in the pathological case, we're still at the mercy of whatever the line p = (IDbDataParameter)command.Parameters[name]; chooses to do; if the parameters are indexed, such that this is not O(N), I would expect Contains to use that index; hence my thinking that if we're going to the effort of creating a hash, we might as well go full hog and create a dictionary.

But I will acknowledge: this only impacts the bad case where parameters are duplicated and are being overwritten. In reality: that will almost never be hit. If you have the code already, I'd be interested in the timings for hashset instead of dictionary, although I don't expect it to be very different.

I guess I'm also philosophically opposed to adding the (albeit minor) allocation overhead in the "it doesn't matter" scenario ;)

[mgravell]

and yes, I know "lets avoid a few allocations" is almost comical when dealing with ADO.NET, which is famously allocation-heavy

[jehhynes]

@mgravell Thanks for the explanation of your approach, I do see the benefits. I guess we had to do the following comparison at some point :-)

If you're interested, here's what I used to generate the comparison:

DynamicParameterFacts.zip

I think one limitation with your approach is you're always evaluating the first 10 checks using the expensive n-squared algo, even if there are more than 10 parameters that will eventually be added and thus you'll eventually have to create a dictionary anyway. I think if you checked parameters.Count instead of checking command.Parameters.Count you could overcome this issue and bring your implementation's performance in line with mine.

As you appear to have specific philosophical ideas about how this ought to be implemented, I'm going to pass this back to you to put in the finishing touches and merge it in. Thanks for your attention with this issue!

[osolliec]

Hello @mgravell @jehhynes, it seems we're also hitting this issue. Do you know if there's something I can do to help merge this PR ?

[jehhynes]

@osolliec I am using the following workaround until this gets merged:

#nullable enable
using System.Data;

namespace Dapper;

/// <summary>
/// A hacked-down version of Dapper.DynamicParameters which solves some performance issues.
/// </summary>
public class SimpleDynamicParameters : SqlMapper.IParameterLookup
{
    internal const DbType EnumerableMultiParameter = (DbType)(-1);
    private readonly Dictionary<string, ParamInfo> parameters = new();

    object? SqlMapper.IParameterLookup.this[string name] =>
        parameters.TryGetValue(name, out ParamInfo? param) ? param.Value : null;

    /// <summary>
    /// Add a parameter to this dynamic parameter list.
    /// </summary>
    /// <param name="name">The name of the parameter.</param>
    /// <param name="value">The value of the parameter.</param>
    /// <param name="dbType">The type of the parameter.</param>
    /// <param name="direction">The in or out direction of the parameter.</param>
    /// <param name="size">The size of the parameter.</param>
    public void Add(string name, object? value, DbType? dbType, ParameterDirection? direction, int? size)
    {
        parameters[Clean(name)] = new ParamInfo
        {
            Name = name,
            Value = value,
            ParameterDirection = direction ?? ParameterDirection.Input,
            DbType = dbType,
            Size = size
        };
    }

    /// <summary>
    /// Add a parameter to this dynamic parameter list.
    /// </summary>
    /// <param name="name">The name of the parameter.</param>
    /// <param name="value">The value of the parameter.</param>
    /// <param name="dbType">The type of the parameter.</param>
    /// <param name="direction">The in or out direction of the parameter.</param>
    /// <param name="size">The size of the parameter.</param>
    /// <param name="precision">The precision of the parameter.</param>
    /// <param name="scale">The scale of the parameter.</param>
    public void Add(string name, object? value = null, DbType? dbType = null, ParameterDirection? direction = null, int? size = null, byte? precision = null, byte? scale = null)
    {
        parameters[Clean(name)] = new ParamInfo
        {
            Name = name,
            Value = value,
            ParameterDirection = direction ?? ParameterDirection.Input,
            DbType = dbType,
            Size = size,
            Precision = precision,
            Scale = scale
        };
    }

    private static string Clean(string name)
    {
        if (!string.IsNullOrEmpty(name))
        {
            switch (name[0])
            {
                case '@':
                case ':':
                case '?':
                    return name.Substring(1);
            }
        }
        return name;
    }

    void SqlMapper.IDynamicParameters.AddParameters(IDbCommand command, SqlMapper.Identity identity)
    {
        AddParameters(command, identity);
    }

    internal static bool ShouldSetDbType(DbType? dbType)
        => dbType.HasValue && dbType.GetValueOrDefault() != EnumerableMultiParameter;

    internal static bool ShouldSetDbType(DbType dbType)
        => dbType != EnumerableMultiParameter; // just in case called with non-nullable

    /// <summary>
    /// Add all the parameters needed to the command just before it executes
    /// </summary>
    /// <param name="command">The raw command prior to execution</param>
    /// <param name="identity">Information about the query</param>
    protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
    {
        foreach (var param in parameters.Values)
        {
            var dbType = param.DbType;
            var val = param.Value;
            string name = Clean(param.Name);
            var isCustomQueryParameter = val is SqlMapper.ICustomQueryParameter;

            SqlMapper.ITypeHandler? handler = null;
            if (dbType is null && val is not null && !isCustomQueryParameter)
            {
#pragma warning disable 618
                dbType = SqlMapper.LookupDbType(val.GetType(), name, true, out handler);
#pragma warning disable 618
            }

            if (isCustomQueryParameter)
            {
                ((SqlMapper.ICustomQueryParameter)val!).AddParameter(command, name);
            }
            else if (dbType == EnumerableMultiParameter)
            {
#pragma warning disable 612, 618
                SqlMapper.PackListParameters(command, name, val);
#pragma warning restore 612, 618
            }
            else
            {
                IDbDataParameter p = command.CreateParameter();
                p.ParameterName = name;

                p.Direction = param.ParameterDirection;
                if (handler is null)
                {
#pragma warning disable 0618
                    p.Value = SqlMapper.SanitizeParameterValue(val);
#pragma warning restore 0618
                    if (ShouldSetDbType(dbType) && p.DbType != dbType.GetValueOrDefault())
                    {
                        p.DbType = dbType.GetValueOrDefault();
                    }
                    var s = val as string;
                    if (s?.Length <= DbString.DefaultLength)
                    {
                        p.Size = DbString.DefaultLength;
                    }
                    if (param.Size is not null) p.Size = param.Size.Value;
                    if (param.Precision is not null) p.Precision = param.Precision.Value;
                    if (param.Scale is not null) p.Scale = param.Scale.Value;
                }
                else
                {
                    if (ShouldSetDbType(dbType)) p.DbType = dbType.GetValueOrDefault();
                    if (param.Size is not null) p.Size = param.Size.Value;
                    if (param.Precision is not null) p.Precision = param.Precision.Value;
                    if (param.Scale is not null) p.Scale = param.Scale.Value;
                    handler.SetValue(p, val ?? DBNull.Value);
                }

                command.Parameters.Add(p);
                param.AttachedParam = p;
            }
        }
    }

    /// <summary>
    /// All the names of the param in the bag, use Get to yank them out
    /// </summary>
    public IEnumerable<string> ParameterNames => parameters.Select(p => p.Key);

    /// <summary>
    /// Get the value of a parameter
    /// </summary>
    /// <typeparam name="T"></typeparam>
    /// <param name="name"></param>
    /// <returns>The value, note DBNull.Value is not returned, instead the value is returned as null</returns>
    public T Get<T>(string name)
    {
        var paramInfo = parameters[Clean(name)];
        var attachedParam = paramInfo.AttachedParam;
        object? val = attachedParam is null ? paramInfo.Value : attachedParam.Value;
        if (val == DBNull.Value)
        {
            if (default(T) is not null)
            {
                throw new ApplicationException("Attempting to cast a DBNull to a non nullable type! Note that out/return parameters will not have updated values until the data stream completes (after the 'foreach' for Query(..., buffered: false), or after the GridReader has been disposed for QueryMultiple)");
            }
            return default!;
        }
        return (T)val!;
    }

    private sealed class ParamInfo
    {
        public string Name { get; set; } = null!;
        public object? Value { get; set; }
        public ParameterDirection ParameterDirection { get; set; }
        public DbType? DbType { get; set; }
        public int? Size { get; set; }
        public IDbDataParameter AttachedParam { get; set; } = null!;

        public byte? Precision { get; set; }
        public byte? Scale { get; set; }
    }
}