- prefer dictionary over hashset, to avoid additional lookup

- defer the cost so we only start paying it for non-trivial commands
DapperLib · Mar 7, 2024 · ddc6737 · ddc6737
1 parent 01970b5
commit ddc6737
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 20 deletions.
diff --git a/Dapper/DynamicParameters.cs b/Dapper/DynamicParameters.cs
@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Data;
-using System.Globalization;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -223,9 +222,50 @@ protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
                 }
             }
 
-            HashSet<string> addedParameters = new HashSet<string>(command.Parameters.Cast<IDbDataParameter>()
-                .Select(x => x.ParameterName), comparer: StringComparer.CurrentCultureIgnoreCase);
+            static IDbDataParameter GetOrAdd(IDbCommand command, string name, ref Dictionary<string, IDbDataParameter>? lookup)
+            {
+                IDbDataParameter p;
+                if (lookup is null && command.Parameters.Count <= 10)
+                {
+                    // don't pay dictionary overhead for small collections
+                    if (command.Parameters.Contains(name))
+                    {
+                        // already exists
+                        p = (IDbDataParameter)command.Parameters[name];
+                    }
+                    else
+                    {
+                        // new
+                        p = command.CreateParameter();
+                        p.ParameterName = name;
+                        command.Parameters.Add(p);
+                    }
+                }
+                else
+                {
+                    if (lookup is null)
+                    {
+                        // build the initial map
+                        lookup = new Dictionary<string, IDbDataParameter>(command.Parameters.Count + 10, StringComparer.CurrentCultureIgnoreCase);
+                        foreach (IDbDataParameter existing in command.Parameters)
+                        {
+                            lookup[existing.ParameterName] = existing;
+                        }
+                    }
 
+                    // add if necessary
+                    if (!lookup.TryGetValue(name, out p!))
+                    {
+                        p = command.CreateParameter();
+                        p.ParameterName = name;
+                        lookup.Add(name, p);
+                        command.Parameters.Add(p);
+                    }
+                }
+                return p;
+            }
+
+            Dictionary<string, IDbDataParameter>? paramLookup = null;
             foreach (var param in parameters.Values)
             {
                 if (param.CameFromTemplate) continue;
@@ -254,17 +294,7 @@ protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
                 }
                 else
                 {
-                    bool add = !addedParameters.Contains(name);
-                    IDbDataParameter p;
-                    if (add)
-                    {
-                        p = command.CreateParameter();
-                        p.ParameterName = name;
-                    }
-                    else
-                    {
-                        p = (IDbDataParameter)command.Parameters[name];
-                    }
+                    var p = GetOrAdd(command, name, ref paramLookup);
 
                     p.Direction = param.ParameterDirection;
                     if (handler is null)
@@ -294,14 +324,8 @@ protected void AddParameters(IDbCommand command, SqlMapper.Identity identity)
                         handler.SetValue(p, val ?? DBNull.Value);
                     }
 
-                    if (add)
-                    {
-                        command.Parameters.Add(p);
-                    }
                     param.AttachedParam = p;
                 }
-
-                addedParameters.Add(name);
             }
 
             // note: most non-privileged implementations would use: this.ReplaceLiterals(command);

diff --git a/tests/Dapper.Tests/ParameterTests.cs b/tests/Dapper.Tests/ParameterTests.cs
@@ -9,6 +9,7 @@
 using System.Dynamic;
 using System.Globalization;
 using System.Linq;
+using System.Text;
 using System.Text.RegularExpressions;
 using Xunit;
 
@@ -1136,6 +1137,32 @@ public void TestSupportForDynamicParametersOutputExpressions_Query_Buffered()
             }
         }
 
+        [Theory]
+        [InlineData(0)]
+        [InlineData(1)]
+        [InlineData(5)]
+        [InlineData(9)]
+        [InlineData(10)]
+        [InlineData(11)]
+        [InlineData(150)]
+        [InlineData(1500)]
+
+        public void DynamicParameterNumber(int count)
+        {
+            // checking no oddities adding different numbers of dynamic parameters
+            int expected = 0;
+            var sql = new StringBuilder("select 0");
+            var args = new DynamicParameters();
+            for (int i = 1; i <= count; i++)
+            {
+                expected += i;
+                sql.Append("+@a").Append(i);
+                args.Add("a" + i, i);
+            }
+            var actual = connection.QuerySingle<int>(sql.ToString(), args);
+            Assert.Equal(expected, actual);
+        }
+
         [Fact]
         public void TestSupportForDynamicParametersOutputExpressions_Query_NonBuffered()
         {