Reimplement MaxBatchSize as a pre-check

For better perf on SQLite (#27681)

Also moves the handling of MaxBatchSize to
ReaderModificationCommandBatch and does some cleanup.

src/EFCore.Relational/Update/ModificationCommandBatch.cs

@@ -56,7 +56,5 @@ public abstract class ModificationCommandBatch
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <returns>A task that represents the asynchronous save operation.</returns>
     /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
-    public abstract Task ExecuteAsync(
-        IRelationalConnection connection,
-        CancellationToken cancellationToken = default);
+    public abstract Task ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken = default);
 }

src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs

@@ -23,9 +23,9 @@ public abstract class ReaderModificationCommandBatch : ModificationCommandBatch
 {
     private readonly List<IReadOnlyModificationCommand> _modificationCommands = new();
     private readonly int _batchHeaderLength;
-    private readonly List<string> _pendingParameterNames = new();
     private bool _requiresTransaction = true;
     private int _sqlBuilderPosition, _commandResultSetCount, _resultsPositionalMappingEnabledLength;
+    private int _pendingParameters;
 
     /// <summary>
     ///     Creates a new <see cref="ReaderModificationCommandBatch" /> instance.
@@ -57,6 +57,12 @@ protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependen
     /// </summary>
     protected virtual IRelationalCommandBuilder RelationalCommandBuilder { get; }
 
+    /// <summary>
+    ///     The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch.
+    /// </summary>
+    protected virtual int MaxBatchSize
+        => 1000;
+
     /// <summary>
     ///     Gets the command text builder for the commands in the batch.
     /// </summary>
@@ -98,9 +104,14 @@ public override bool TryAddCommand(IReadOnlyModificationCommand modificationComm
             throw new InvalidOperationException(RelationalStrings.ModificationCommandBatchAlreadyComplete);
         }
 
+        if (_modificationCommands.Count >= MaxBatchSize)
+        {
+            return false;
+        }
+
         _sqlBuilderPosition = SqlBuilder.Length;
         _commandResultSetCount = CommandResultSet.Count;
-        _pendingParameterNames.Clear();
+        _pendingParameters = 0;
         _resultsPositionalMappingEnabledLength = ResultsPositionalMappingEnabled?.Length ?? 0;
 
         AddCommand(modificationCommand);
@@ -144,11 +155,13 @@ protected virtual void RollbackLastCommand()
             ResultsPositionalMappingEnabled.Length = _resultsPositionalMappingEnabledLength;
         }
 
-        foreach (var pendingParameterName in _pendingParameterNames)
+        for (var i = 0; i < _pendingParameters; i++)
         {
-            ParameterValues.Remove(pendingParameterName);
+            var parameterIndex = RelationalCommandBuilder.Parameters.Count - 1;
+            var parameter = RelationalCommandBuilder.Parameters[i];
 
-            RelationalCommandBuilder.RemoveParameterAt(RelationalCommandBuilder.Parameters.Count - 1);
+            RelationalCommandBuilder.RemoveParameterAt(parameterIndex);
+            ParameterValues.Remove(parameter.InvariantName);
         }
     }
 
@@ -173,7 +186,8 @@ protected virtual void SetRequiresTransaction(bool requiresTransaction)
     ///     Checks whether the command text is valid.
     /// </summary>
     /// <returns><see langword="true" /> if the command text is valid; <see langword="false" /> otherwise.</returns>
-    protected abstract bool IsValid();
+    protected virtual bool IsValid()
+        => true;
 
     /// <summary>
     ///     Adds Updates the command text for the command at the given position in the <see cref="ModificationCommands" /> list.
@@ -265,7 +279,7 @@ protected virtual void AddParameter(IColumnModification columnModification)
 
             ParameterValues.Add(columnModification.ParameterName, columnModification.Value);
 
-            _pendingParameterNames.Add(columnModification.ParameterName);
+            _pendingParameters++;
         }
 
         if (columnModification.UseOriginalValueParameter)
@@ -278,7 +292,7 @@ protected virtual void AddParameter(IColumnModification columnModification)
 
             ParameterValues.Add(columnModification.OriginalParameterName, columnModification.OriginalValue);
 
-            _pendingParameterNames.Add(columnModification.OriginalParameterName);
+            _pendingParameters++;
         }
     }
 

src/EFCore.Relational/Update/SingularModificationCommandBatch.cs

@@ -28,11 +28,8 @@ public SingularModificationCommandBatch(ModificationCommandBatchFactoryDependenc
     }
 
     /// <summary>
-    ///     Returns <see langword="true" /> only when the batch contains a single command.
+    ///     The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch; always returns 1.
     /// </summary>
-    /// <returns>
-    ///     <see langword="true" /> when the batch contains a single command, <see langword="false" /> otherwise.
-    /// </returns>
-    protected override bool IsValid()
-        => ModificationCommands.Count == 1;
+    protected override int MaxBatchSize
+        => 1;
 }

src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs

@@ -18,7 +18,6 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman
     private const int DefaultNetworkPacketSizeBytes = 4096;
     private const int MaxScriptLength = 65536 * DefaultNetworkPacketSizeBytes / 2;
     private const int MaxParameterCount = 2100;
-    private const int MaxRowCount = 1000;
     private readonly int _maxBatchSize;
     private readonly List<IReadOnlyModificationCommand> _pendingBulkInsertCommands = new();
 
@@ -30,16 +29,9 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman
     /// </summary>
     public SqlServerModificationCommandBatch(
         ModificationCommandBatchFactoryDependencies dependencies,
-        int? maxBatchSize)
+        int maxBatchSize)
         : base(dependencies)
-    {
-        if (maxBatchSize is <= 0)
-        {
-            throw new ArgumentOutOfRangeException(nameof(maxBatchSize), RelationalStrings.InvalidMaxBatchSize(maxBatchSize.Value));
-        }
-
-        _maxBatchSize = Math.Min(maxBatchSize ?? 42, MaxRowCount);
-    }
+        => _maxBatchSize = maxBatchSize;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -50,6 +42,15 @@ public SqlServerModificationCommandBatch(
     protected new virtual ISqlServerUpdateSqlGenerator UpdateSqlGenerator
         => (ISqlServerUpdateSqlGenerator)base.UpdateSqlGenerator;
 
+    /// <summary>
+    ///     The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch.
+    /// </summary>
+    /// <remarks>
+    ///     For SQL Server, this is 42 by default, and cannot exceed 1000.
+    /// </remarks>
+    protected override int MaxBatchSize
+        => _maxBatchSize;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -74,8 +75,7 @@ protected override void RollbackLastCommand()
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     protected override bool IsValid()
-        => ModificationCommands.Count <= _maxBatchSize
-            && SqlBuilder.Length < MaxScriptLength
+        => SqlBuilder.Length < MaxScriptLength
             // A single implicit parameter for the command text itself
             && ParameterValues.Count + 1 < MaxParameterCount;
 

src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatchFactory.cs

@@ -13,7 +13,9 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal;
 /// </summary>
 public class SqlServerModificationCommandBatchFactory : IModificationCommandBatchFactory
 {
-    private readonly IDbContextOptions _options;
+    private const int DefaultMaxBatchSize = 42;
+    private const int MaxMaxBatchSize = 1000;
+    private readonly int _maxBatchSize;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -26,7 +28,16 @@ public SqlServerModificationCommandBatchFactory(
         IDbContextOptions options)
     {
         Dependencies = dependencies;
-        _options = options;
+
+        _maxBatchSize = Math.Min(
+            options.Extensions.OfType<SqlServerOptionsExtension>().FirstOrDefault()?.MaxBatchSize ?? DefaultMaxBatchSize,
+            MaxMaxBatchSize);
+
+        if (_maxBatchSize <= 0)
+        {
+            throw new ArgumentOutOfRangeException(
+                nameof(RelationalOptionsExtension.MaxBatchSize), RelationalStrings.InvalidMaxBatchSize(_maxBatchSize));
+        }
     }
 
     /// <summary>
@@ -41,9 +52,5 @@ public SqlServerModificationCommandBatchFactory(
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual ModificationCommandBatch Create()
-    {
-        var optionsExtension = _options.Extensions.OfType<SqlServerOptionsExtension>().FirstOrDefault();
-
-        return new SqlServerModificationCommandBatch(Dependencies, optionsExtension?.MaxBatchSize);
-    }
+        => new SqlServerModificationCommandBatch(Dependencies, _maxBatchSize);
 }

test/EFCore.Relational.Tests/TestUtilities/TestModificationCommandBatch.cs

@@ -5,16 +5,11 @@ namespace Microsoft.EntityFrameworkCore.TestUtilities;
 
 public class TestModificationCommandBatch : SingularModificationCommandBatch
 {
-    private readonly int _maxBatchSize;
-
     public TestModificationCommandBatch(
         ModificationCommandBatchFactoryDependencies dependencies,
         int? maxBatchSize)
         : base(dependencies)
-    {
-        _maxBatchSize = maxBatchSize ?? 1;
-    }
+        => MaxBatchSize = maxBatchSize ?? 1;
 
-    protected override bool IsValid()
-        => ModificationCommands.Count <= _maxBatchSize;
+    protected override int MaxBatchSize { get; }
 }

test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs

@@ -125,6 +125,9 @@ public void AddCommand_does_not_add_command_batch_is_invalid()
 ",
             batch.CommandText,
             ignoreLineEndingDifferences: true);
+
+        Assert.Equal(1, batch.StoreCommand.RelationalCommand.Parameters.Count);
+        Assert.Equal(1, batch.StoreCommand.ParameterValues.Count);
     }
 
     [ConditionalFact]

test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs

@@ -61,7 +61,7 @@ private class FakeDbContext : DbContext
     {
     }
 
-    private static TestSqlServerModificationCommandBatch CreateBatch(int? maxBatchSize = null)
+    private static TestSqlServerModificationCommandBatch CreateBatch(int maxBatchSize = 42)
     {
         var typeMapper = CreateTypeMappingSource();
 
@@ -100,7 +100,7 @@ private static IModificationCommand CreateModificationCommand(
 
     private class TestSqlServerModificationCommandBatch : SqlServerModificationCommandBatch
     {
-        public TestSqlServerModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies, int? maxBatchSize)
+        public TestSqlServerModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies, int maxBatchSize)
             : base(dependencies, maxBatchSize)
         {
         }