From 2de9e38bf35b97bb88ceb6126087aadb6dbc302b Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Wed, 6 Feb 2019 18:18:38 +0200 Subject: [PATCH 1/4] Skip Topological sorting if not required --- src/NHibernate/Loader/JoinWalker.cs | 30 +++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index f15c220d6dc..bb9aecd97ab 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -204,8 +204,14 @@ protected virtual SelectMode GetSelectMode(string path) return SelectMode.Undefined; } + /// + /// Returns null if sorting is not required, otherwise list of indexes in sorted order + /// private static int[] GetTopologicalSortOrder(List fields) { + if (!fields.Exists(a => a.DependsOn?.Length > 0)) + return null; + TopologicalSorter g = new TopologicalSorter(fields.Count); Dictionary indexes = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -799,16 +805,9 @@ protected SqlString MergeOrderings(string ass, string orderBy) { /// protected JoinFragment MergeOuterJoins(IList associations) { - IList sortedAssociations = new List(); - - var indices = GetTopologicalSortOrder(_dependentAliases); - for (int index = indices.Length - 1; index >= 0; index--) - { - sortedAssociations.Add(associations[indices[index]]); - } - JoinFragment outerjoin = Dialect.CreateOuterJoinFragment(); + var sortedAssociations = GetSortedAssociations(associations); OuterJoinableAssociation last = null; foreach (OuterJoinableAssociation oj in sortedAssociations) { @@ -840,6 +839,21 @@ protected JoinFragment MergeOuterJoins(IList associati return outerjoin; } + private IList GetSortedAssociations(IList associations) + { + var indexes = GetTopologicalSortOrder(_dependentAliases); + if (indexes == null) + return associations; + + var sortedAssociations = new List(associations.Count); + for (int index = indexes.Length - 1; index >= 0; index--) + { + sortedAssociations.Add(associations[indexes[index]]); + } + + return sortedAssociations; + } + /// /// Count the number of instances of IJoinable which are actually /// also instances of ILoadable, or are one-to-many associations From 1f9a7cf869dd0294d0c4bb935fe93ab8423ce1d9 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 7 Feb 2019 06:20:47 +0200 Subject: [PATCH 2/4] Move dependent alias calculation to sort method --- src/NHibernate/Loader/JoinWalker.cs | 62 +++++++++++++++++------------ 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index bb9aecd97ab..7bfecd51e0e 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -121,8 +121,6 @@ public class DependentAlias public string[] DependsOn { get; set; } } - readonly List _dependentAliases = new List(); - protected JoinWalker(ISessionFactoryImplementor factory, IDictionary enabledFilters) { this.factory = factory; @@ -181,7 +179,7 @@ private void AddAssociationToJoinTree(IAssociationType type, string[] aliasedLhs enabledFilters, GetSelectMode(path)); assoc.ValidateJoin(path); - AddAssociation(subalias, assoc); + AddAssociation(assoc); int nextDepth = currentDepth + 1; @@ -207,13 +205,17 @@ protected virtual SelectMode GetSelectMode(string path) /// /// Returns null if sorting is not required, otherwise list of indexes in sorted order /// - private static int[] GetTopologicalSortOrder(List fields) + private static int[] GetTopologicalSortOrder(IList associations) { + if (associations.Count < 2) + return null; + + var fields = GetDependentAliases(associations); if (!fields.Exists(a => a.DependsOn?.Length > 0)) return null; TopologicalSorter g = new TopologicalSorter(fields.Count); - Dictionary indexes = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary indexes = new Dictionary(fields.Count, StringComparer.OrdinalIgnoreCase); // add vertices for (int i = 0; i < fields.Count; i++) @@ -242,31 +244,41 @@ private static int[] GetTopologicalSortOrder(List fields) return g.Sort(); } - /// - /// Adds an association and extracts the aliases the association's 'with clause' is dependent on - /// - private void AddAssociation(string subalias, OuterJoinableAssociation association) + private static List GetDependentAliases(IList associations) { - var dependentAlias = new DependentAlias - { - Alias = subalias, - }; - _dependentAliases.Add(dependentAlias); + var dependentAliases = new List(associations.Count); - var on = association.On.ToString(); - if (!string.IsNullOrEmpty(on)) + foreach (var association in associations) { - var dependencies = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (Match match in aliasRegex.Matches(on)) + var dependentAlias = new DependentAlias { - string alias = match.Value; - if (string.Equals(alias, subalias, StringComparison.OrdinalIgnoreCase)) - continue; - dependencies.Add(alias); + Alias = association.RHSAlias, + }; + + dependentAliases.Add(dependentAlias); + + var on = association.On.ToString(); + if (!string.IsNullOrEmpty(on)) + { + var dependencies = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (Match match in aliasRegex.Matches(on)) + { + string alias = match.Value; + if (string.Equals(alias, dependentAlias.Alias, StringComparison.OrdinalIgnoreCase)) + continue; + dependencies.Add(alias); + } + dependentAlias.DependsOn = dependencies.ToArray(); } - dependentAlias.DependsOn = dependencies.ToArray(); } + return dependentAliases; + } + /// + /// Adds an association + /// + private void AddAssociation(OuterJoinableAssociation association) + { associations.Add(association); } @@ -366,7 +378,7 @@ internal void AddExplicitEntityJoinAssociation( Factory, enabledFilters, GetSelectMode(path)); - AddAssociation(tableAlias, assoc); + AddAssociation(assoc); } private void WalkEntityAssociationTree(IAssociationType associationType, IOuterJoinLoadable persister, @@ -841,7 +853,7 @@ protected JoinFragment MergeOuterJoins(IList associati private IList GetSortedAssociations(IList associations) { - var indexes = GetTopologicalSortOrder(_dependentAliases); + var indexes = GetTopologicalSortOrder(associations); if (indexes == null) return associations; From 34b0d5124cd0edbb108a1d19c9f861748e6caf78 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Thu, 7 Feb 2019 07:50:44 +0200 Subject: [PATCH 3/4] remove null-drive logic --- src/NHibernate/Loader/JoinWalker.cs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index 7bfecd51e0e..fc3494bcee6 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -203,17 +203,10 @@ protected virtual SelectMode GetSelectMode(string path) } /// - /// Returns null if sorting is not required, otherwise list of indexes in sorted order + /// Returns list of indexes in sorted order /// - private static int[] GetTopologicalSortOrder(IList associations) + private static int[] GetTopologicalSortOrder(IList fields) { - if (associations.Count < 2) - return null; - - var fields = GetDependentAliases(associations); - if (!fields.Exists(a => a.DependsOn?.Length > 0)) - return null; - TopologicalSorter g = new TopologicalSorter(fields.Count); Dictionary indexes = new Dictionary(fields.Count, StringComparer.OrdinalIgnoreCase); @@ -851,12 +844,16 @@ protected JoinFragment MergeOuterJoins(IList associati return outerjoin; } - private IList GetSortedAssociations(IList associations) + private static IList GetSortedAssociations(IList associations) { - var indexes = GetTopologicalSortOrder(associations); - if (indexes == null) + if (associations.Count < 2) + return associations; + + var fields = GetDependentAliases(associations); + if (!fields.Exists(a => a.DependsOn?.Length > 0)) return associations; + var indexes = GetTopologicalSortOrder(fields); var sortedAssociations = new List(associations.Count); for (int index = indexes.Length - 1; index >= 0; index--) { From f36f5bc3ca2df12e8dcad0ad7a0fd8852208efe8 Mon Sep 17 00:00:00 2001 From: Alexander Zaytsev Date: Sat, 9 Feb 2019 10:02:39 +1300 Subject: [PATCH 4/4] Remove unnecessary .ToArray() call and use DependentAlias2 struct to reduce allocations --- src/NHibernate/Loader/JoinWalker.cs | 71 +++++++++++++++++------------ 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index fc3494bcee6..5e1398f4607 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -115,6 +115,8 @@ protected virtual bool IsTooManyCollections get { return false; } } + //Since v5.3 + [Obsolete("This class is not used and will be removed in a future version.")] public class DependentAlias { public string Alias { get; set; } @@ -202,10 +204,22 @@ protected virtual SelectMode GetSelectMode(string path) return SelectMode.Undefined; } + private struct DependentAlias2 + { + public DependentAlias2(string alias, ICollection dependsOn) + { + Alias = alias; + DependsOn = dependsOn; + } + + public string Alias { get; } + public ICollection DependsOn { get; } + } + /// /// Returns list of indexes in sorted order /// - private static int[] GetTopologicalSortOrder(IList fields) + private static int[] GetTopologicalSortOrder(IList fields) { TopologicalSorter g = new TopologicalSorter(fields.Count); Dictionary indexes = new Dictionary(fields.Count, StringComparer.OrdinalIgnoreCase); @@ -219,14 +233,12 @@ private static int[] GetTopologicalSortOrder(IList fields) // add edges for (int i = 0; i < fields.Count; i++) { - var dependentAlias = fields[i]; - if (dependentAlias.DependsOn != null) + var dependentFields = fields[i].DependsOn; + if (dependentFields != null) { - for (int j = 0; j < dependentAlias.DependsOn.Length; j++) + foreach (var dependentField in dependentFields) { - var dependentField = dependentAlias.DependsOn[j]; - int end; - if (indexes.TryGetValue(dependentField, out end)) + if (indexes.TryGetValue(dependentField, out var end)) { g.AddEdge(i, end); } @@ -237,34 +249,33 @@ private static int[] GetTopologicalSortOrder(IList fields) return g.Sort(); } - private static List GetDependentAliases(IList associations) + private static List GetDependentAliases(IList associations) { - var dependentAliases = new List(associations.Count); - + var dependentAliases = new List(associations.Count); foreach (var association in associations) { - var dependentAlias = new DependentAlias - { - Alias = association.RHSAlias, - }; + dependentAliases.Add(new DependentAlias2(association.RHSAlias, GetDependsOn(association))); + } + + return dependentAliases; + } - dependentAliases.Add(dependentAlias); + private static HashSet GetDependsOn(OuterJoinableAssociation association) + { + if (SqlStringHelper.IsEmpty(association.On)) + return null; - var on = association.On.ToString(); - if (!string.IsNullOrEmpty(on)) - { - var dependencies = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (Match match in aliasRegex.Matches(on)) - { - string alias = match.Value; - if (string.Equals(alias, dependentAlias.Alias, StringComparison.OrdinalIgnoreCase)) - continue; - dependencies.Add(alias); - } - dependentAlias.DependsOn = dependencies.ToArray(); - } + var dependencies = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (Match match in aliasRegex.Matches(association.On.ToString())) + { + string alias = match.Value; + if (string.Equals(alias, association.RHSAlias, StringComparison.OrdinalIgnoreCase)) + continue; + + dependencies.Add(alias); } - return dependentAliases; + + return dependencies; } /// @@ -850,7 +861,7 @@ private static IList GetSortedAssociations(IList a.DependsOn?.Length > 0)) + if (!fields.Exists(a => a.DependsOn?.Count > 0)) return associations; var indexes = GetTopologicalSortOrder(fields);