From c6a9f1c60050db345c28b778239659dcd078e203 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Mon, 20 Sep 2021 10:21:20 -0700 Subject: [PATCH] When sorting update commands add Deleted + Inserted number of edges instead of Deleted * Inserted Don't allocate a collection for edges if there's only one edge between given two vertices Reuse the Multigraph instance Change the benchmark to reuse the context instance to simulate pooling behavior --- .../SimpleUpdatePipelineTests.cs | 4 +- .../Update/Internal/CommandBatchPreparer.cs | 138 +++++++++++++----- src/Shared/Multigraph.cs | 50 ++++--- test/EFCore.Tests/Utilities/MultigraphTest.cs | 3 - 4 files changed, 131 insertions(+), 64 deletions(-) diff --git a/benchmark/EFCore.Benchmarks/UpdatePipeline/SimpleUpdatePipelineTests.cs b/benchmark/EFCore.Benchmarks/UpdatePipeline/SimpleUpdatePipelineTests.cs index b683c4cbab7..268f1fa2655 100644 --- a/benchmark/EFCore.Benchmarks/UpdatePipeline/SimpleUpdatePipelineTests.cs +++ b/benchmark/EFCore.Benchmarks/UpdatePipeline/SimpleUpdatePipelineTests.cs @@ -36,11 +36,11 @@ public virtual void InitializeFixture() { _fixture = CreateFixture(); _fixture.Initialize(0, 1000, 0, 0); + _context = _fixture.CreateContext(disableBatching: Batching); } public virtual void InitializeContext() { - _context = _fixture.CreateContext(disableBatching: Batching); _transaction = _context.Database.BeginTransaction(); } @@ -53,7 +53,7 @@ public virtual void CleanupContext() } _transaction.Dispose(); - _context.Dispose(); + _context.ChangeTracker.Clear(); } [Benchmark] diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 07f1ec9ca97..305f8087876 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -32,6 +32,7 @@ public class CommandBatchPreparer : ICommandBatchPreparer { private readonly int _minBatchSize; private readonly bool _sensitiveLoggingEnabled; + private readonly Multigraph _modificationCommandGraph = new(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -278,18 +279,18 @@ private void AddUnchangedSharingEntries( /// protected virtual IReadOnlyList> TopologicalSort(IEnumerable commands) { - var modificationCommandGraph = new Multigraph(); - modificationCommandGraph.AddVertices(commands); + _modificationCommandGraph.Clear(); + _modificationCommandGraph.AddVertices(commands); // The predecessors map allows to populate the graph in linear time - var predecessorsMap = CreateKeyValuePredecessorMap(modificationCommandGraph); - AddForeignKeyEdges(modificationCommandGraph, predecessorsMap); + var predecessorsMap = CreateKeyValuePredecessorMap(_modificationCommandGraph); + AddForeignKeyEdges(_modificationCommandGraph, predecessorsMap); - AddUniqueValueEdges(modificationCommandGraph); + AddUniqueValueEdges(_modificationCommandGraph); - AddSameTableEdges(modificationCommandGraph); + AddSameTableEdges(_modificationCommandGraph); - return modificationCommandGraph.BatchingTopologicalSort(static (_, _, edges) => edges.All(e => e is IEntityType), FormatCycle); + return _modificationCommandGraph.BatchingTopologicalSort(static (_, _, edges) => edges.All(e => e is IEntityType), FormatCycle); } private string FormatCycle(IReadOnlyList>> data) @@ -488,12 +489,8 @@ private Dictionary> CreateKey var entry = command.Entries[i]; foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) { - var constraints = foreignKey.GetMappedConstraints() - .Where(c => c.PrincipalTable.Name == command.TableName && c.PrincipalTable.Schema == command.Schema); - - if (!constraints.Any() - || (entry.EntityState == EntityState.Modified - && !foreignKey.PrincipalKey.Properties.Any(p => entry.IsModified(p)))) + if (!IsMapped(foreignKey, command, principal: true) + || !IsModified(foreignKey.PrincipalKey.Properties, entry)) { continue; } @@ -523,12 +520,8 @@ private Dictionary> CreateKey { foreach (var foreignKey in entry.EntityType.GetForeignKeys()) { - var constraints = foreignKey.GetMappedConstraints() - .Where(c => c.Table.Name == command.TableName && c.Table.Schema == command.Schema); - - if (!constraints.Any() - || (entry.EntityState == EntityState.Modified - && !foreignKey.Properties.Any(p => entry.IsModified(p)))) + if (!IsMapped(foreignKey, command, principal: false) + || !IsModified(foreignKey.Properties, entry)) { continue; } @@ -571,10 +564,8 @@ private void AddForeignKeyEdges( var entry = command.Entries[entryIndex]; foreach (var foreignKey in entry.EntityType.GetForeignKeys()) { - if (!foreignKey.GetMappedConstraints() - .Any(c => c.Table.Name == command.TableName && c.Table.Schema == command.Schema) - || (entry.EntityState == EntityState.Modified - && !foreignKey.Properties.Any(p => entry.IsModified(p)))) + if (!IsMapped(foreignKey, command, principal: false) + || !IsModified(foreignKey.Properties, entry)) { continue; } @@ -600,9 +591,7 @@ private void AddForeignKeyEdges( var entry = command.Entries[entryIndex]; foreach (var foreignKey in entry.EntityType.GetReferencingForeignKeys()) { - var constraints = foreignKey.GetMappedConstraints() - .Where(c => c.PrincipalTable.Name == command.TableName && c.PrincipalTable.Schema == command.Schema); - if (!constraints.Any()) + if (!IsMapped(foreignKey, command, principal: true)) { continue; } @@ -623,6 +612,49 @@ private void AddForeignKeyEdges( } } + private static bool IsMapped(IForeignKey foreignKey, IReadOnlyModificationCommand command, bool principal) + { + foreach (var constraint in foreignKey.GetMappedConstraints()) + { + if (principal) + { + if (constraint.PrincipalTable.Name == command.TableName + && constraint.PrincipalTable.Schema == command.Schema) + { + return true; + } + } + else + { + if (constraint.Table.Name == command.TableName + && constraint.Table.Schema == command.Schema) + { + return true; + } + } + } + + return false; + } + + private static bool IsModified(IReadOnlyList properties, IUpdateEntry entry) + { + if (entry.EntityState != EntityState.Modified) + { + return true; + } + + foreach (var property in properties) + { + if (entry.IsModified(property)) + { + return true; + } + } + + return false; + } + private static void AddMatchingPredecessorEdge( Dictionary> predecessorsMap, T keyValue, @@ -658,10 +690,11 @@ private void AddUniqueValueEdges(Multigraph i.IsUnique && i.GetMappedTableIndexes().Any())) + foreach (var index in entry.EntityType.GetIndexes()) { - if (entry.EntityState == EntityState.Modified - && !index.Properties.Any(p => entry.IsModified(p))) + if (!index.IsUnique + || !index.GetMappedTableIndexes().Any() + || !IsModified(index.Properties, entry)) { continue; } @@ -688,8 +721,13 @@ private void AddUniqueValueEdges(Multigraph k.GetMappedConstraints().Any())) + foreach (var key in entry.EntityType.GetKeys()) { + if (!key.GetMappedConstraints().Any()) + { + continue; + } + var principalKeyValue = Dependencies.KeyValueIndexFactorySource .GetKeyValueIndexFactory(key) .CreatePrincipalKeyValue(entry, null); @@ -719,10 +757,11 @@ private void AddUniqueValueEdges(Multigraph i.IsUnique && i.GetMappedTableIndexes().Any())) + foreach (var index in entry.EntityType.GetIndexes()) { - if (entry.EntityState == EntityState.Modified - && !index.Properties.Any(p => entry.IsModified(p))) + if (!index.IsUnique + || !index.GetMappedTableIndexes().Any() + || !IsModified(index.Properties, entry)) { continue; } @@ -751,8 +790,13 @@ private void AddUniqueValueEdges(Multigraph k.GetMappedConstraints().Any())) + foreach (var key in entry.EntityType.GetKeys()) { + if (!key.GetMappedConstraints().Any()) + { + continue; + } + var principalKeyValue = Dependencies.KeyValueIndexFactorySource .GetKeyValueIndexFactory(key) .CreatePrincipalKeyValue(entry, null); @@ -770,13 +814,19 @@ private void AddUniqueValueEdges(Multigraph modificationCommandGraph) { - var deletedDictionary = new Dictionary<(string, string?), List>(); + var deletedDictionary = new Dictionary<(string, string?), (List List, bool EdgesAdded)>(); foreach (var command in modificationCommandGraph.Vertices) { if (command.EntityState == EntityState.Deleted) { - deletedDictionary.GetOrAddNew((command.TableName, command.Schema)).Add(command); + var table = (command.TableName, command.Schema); + if (!deletedDictionary.TryGetValue(table, out var deletedCommands)) + { + deletedCommands = (new List(), false); + deletedDictionary.Add(table, deletedCommands); + } + deletedCommands.List.Add(command); } } @@ -784,12 +834,22 @@ private static void AddSameTableEdges(Multigraph : Graph where TVertex : notnull { private readonly HashSet _vertices = new(); - private readonly Dictionary>> _successorMap = new(); + private readonly Dictionary> _successorMap = new(); private readonly Dictionary> _predecessorMap = new(); - public IEnumerable Edges - => _successorMap.Values.SelectMany(s => s.Values).SelectMany(e => e).Distinct(); - public IEnumerable GetEdges(TVertex from, TVertex to) { if (_successorMap.TryGetValue(from, out var successorSet)) { - if (successorSet.TryGetValue(to, out var edgeList)) + if (successorSet.TryGetValue(to, out var edges)) { - return edgeList; + return edges is IEnumerable edgeList ? edgeList : (new TEdge[] { (TEdge)edges! }); } } @@ -55,17 +52,23 @@ public void AddEdge(TVertex from, TVertex to, TEdge edge) if (!_successorMap.TryGetValue(from, out var successorEdges)) { - successorEdges = new Dictionary>(); + successorEdges = new Dictionary(); _successorMap.Add(from, successorEdges); } - if (!successorEdges.TryGetValue(to, out var edgeList)) + if (successorEdges.TryGetValue(to, out var edges)) { - edgeList = new List(); - successorEdges.Add(to, edgeList); + if (edges is not List edgeList) + { + edgeList = new List { (TEdge)edges! }; + successorEdges[to] = edgeList; + } + edgeList.Add(edge); + } + else + { + successorEdges.Add(to, edge); } - - edgeList.Add(edge); if (!_predecessorMap.TryGetValue(to, out var predecessors)) { @@ -76,7 +79,7 @@ public void AddEdge(TVertex from, TVertex to, TEdge edge) predecessors.Add(from); } - public void AddEdges(TVertex from, TVertex to, IEnumerable edges) + public void AddEdges(TVertex from, TVertex to, IEnumerable newEdges) { #if DEBUG if (!_vertices.Contains(from)) @@ -92,18 +95,25 @@ public void AddEdges(TVertex from, TVertex to, IEnumerable edges) if (!_successorMap.TryGetValue(from, out var successorEdges)) { - successorEdges = new Dictionary>(); + successorEdges = new Dictionary(); _successorMap.Add(from, successorEdges); } - if (!successorEdges.TryGetValue(to, out var edgeList)) + if (successorEdges.TryGetValue(to, out var edges)) { - edgeList = new List(); + if (edges is not List edgeList) + { + edgeList = new List { (TEdge)edges! }; + successorEdges[to] = edgeList; + } + edgeList.AddRange(newEdges); + } + else + { + var edgeList = newEdges.ToList(); successorEdges.Add(to, edgeList); } - edgeList.AddRange(edges); - if (!_predecessorMap.TryGetValue(to, out var predecessors)) { predecessors = new HashSet(); @@ -193,7 +203,7 @@ public IReadOnlyList TopologicalSort( .First(neighbor => predecessorCounts.TryGetValue(neighbor, out var neighborPredecessors) && neighborPredecessors > 0); - if (tryBreakEdge(incomingNeighbor, candidateVertex, _successorMap[incomingNeighbor][candidateVertex])) + if (tryBreakEdge(incomingNeighbor, candidateVertex, GetEdges(incomingNeighbor, candidateVertex))) { _successorMap[incomingNeighbor].Remove(candidateVertex); _predecessorMap[candidateVertex].Remove(incomingNeighbor); @@ -367,7 +377,7 @@ public IReadOnlyList> BatchingTopologicalSort( .First(neighbor => predecessorCounts.TryGetValue(neighbor, out var neighborPredecessors) && neighborPredecessors > 0); - if (tryBreakEdge(incomingNeighbor, candidateVertex, _successorMap[incomingNeighbor][candidateVertex])) + if (tryBreakEdge(incomingNeighbor, candidateVertex, GetEdges(incomingNeighbor, candidateVertex))) { _successorMap[incomingNeighbor].Remove(candidateVertex); _predecessorMap[candidateVertex].Remove(incomingNeighbor); diff --git a/test/EFCore.Tests/Utilities/MultigraphTest.cs b/test/EFCore.Tests/Utilities/MultigraphTest.cs index 3310ab14ffe..1075b14d663 100644 --- a/test/EFCore.Tests/Utilities/MultigraphTest.cs +++ b/test/EFCore.Tests/Utilities/MultigraphTest.cs @@ -139,9 +139,6 @@ public void AddEdge_adds_an_edge() graph.AddEdge(vertexOne, vertexTwo, edgeOne); graph.AddEdge(vertexOne, vertexTwo, edgeTwo); - Assert.Equal(2, graph.Edges.Count()); - Assert.Equal(2, graph.Edges.Intersect(new[] { edgeOne, edgeTwo }).Count()); - Assert.Empty(graph.GetEdges(vertexTwo, vertexOne)); Assert.Equal(2, graph.GetEdges(vertexOne, vertexTwo).Count()); Assert.Equal(2, graph.GetEdges(vertexOne, vertexTwo).Intersect(new[] { edgeOne, edgeTwo }).Count());