Merge pull request #186 from servicetitan/upstream/optimizeSomeHotPaths

Optimize some hot paths

Author: alex-kulakov

Orm/Xtensive.Orm/Modelling/Comparison/Comparer.cs

@@ -14,6 +14,7 @@
 using Xtensive.Reflection;
 using Xtensive.Modelling.Attributes;
 using Xtensive.Modelling.Comparison.Hints;
+using Xtensive.Orm;
 
 
 namespace Xtensive.Modelling.Comparison
@@ -149,21 +150,27 @@ protected virtual Difference VisitNode(Node source, Node target)
     {
       using (TryActivate(source, target, (s, t) => new NodeDifference(s, t))) {
         IgnoreHint ignoreHint = null;
-        if (source!=null)
+        if (source!=null) {
           ignoreHint = Hints.GetHint<IgnoreHint>(source);
-        if (ignoreHint!=null)
+        }
+
+        if (ignoreHint!=null) {
           return null;
+        }
 
         var context = Context;
         var difference = (NodeDifference) context.Difference;
-        if (difference==null)
+        if (difference==null) {
           throw new NullReferenceException();
+        }
+
         var any = source ?? target;
-        if (any==null)
+        if (any==null) {
           throw Exceptions.InternalError(Strings.ExBothSourceAndTargetAreNull, CoreLog.Instance);
+        }
 
 
-        bool isNewDifference = TryRegisterDifference(source, target, difference);
+        var isNewDifference = TryRegisterDifference(source, target, difference);
         if (isNewDifference) {
           // Build movement info
           difference.MovementInfo = BuildMovementInfo(source, target);
@@ -353,68 +360,74 @@ protected IEnumerable<NodeDifference> GetPropertyDifferences(NodeDifference diff
     /// <exception cref="NullReferenceException">Current difference is not <see cref="NodeCollectionDifference"/>.</exception>
     protected virtual Difference VisitNodeCollection(NodeCollection source, NodeCollection target)
     {
-      using (TryActivate(source, target, (s,t) => new NodeCollectionDifference(s,t))) {
+      using (TryActivate(source, target, (s, t) => new NodeCollectionDifference(s, t))) {
         var context = Context;
         var difference = (NodeCollectionDifference) context.Difference;
-        if (difference==null)
+        if (difference == null) {
           throw new NullReferenceException();
+        }
 
-        bool isNewDifference = TryRegisterDifference(source, target, difference);
+        TryRegisterDifference(source, target, difference);
         difference.ItemChanges.Clear();
 
-        // Inlining 2 below lines leads to error in PEVerify.exe!
-        // (well-known issue with null coalescing operator + cast)
-        var sourceItems = (IEnumerable) source;
-        var targetItems = (IEnumerable) target;
+        if (source?.Count == 0 && target?.Count == 0) {
+          return null;
+        }
+
+        var sourceSize = source?.Count ?? 0;
+        var sourceKeyMap = new Dictionary<string, Node>(sourceSize, StringComparer.OrdinalIgnoreCase);
+        for (var index = sourceSize; index-- > 0;) {
+          var node = source[index];
+          sourceKeyMap.Add(GetNodeComparisonKey(node), node);
+        }
 
-        var src = sourceItems ?? new ReadOnlyList<Node>(new Node[] {});
-        var tgt = targetItems ?? new ReadOnlyList<Node>(new Node[] {});
+        var targetSize = target?.Count ?? 0;
+        var targetKeyMap = new Dictionary<string, Node>(targetSize, StringComparer.OrdinalIgnoreCase);
+        for (var index = targetSize; index-- > 0;) {
+          var node = target[index];
+          targetKeyMap.Add(GetNodeComparisonKey(node), node);
+        }
 
-        var srcCount = source!=null ? source.Count : 0;
-        var tgtCount = target!=null ? target.Count : 0;
+        foreach (var sourceItem in sourceKeyMap) {
+          if (!targetKeyMap.ContainsKey(sourceItem.Key)) {
+            var d = Visit(sourceKeyMap[sourceItem.Key], null);
+            if (d != null) {
+              difference.ItemChanges.Add((NodeDifference) d);
+            }
+          }
+        }
 
-        if (srcCount==0 && tgtCount==0)
-          return null;
-        var someItems = srcCount!=0 ? src : tgt;
-        var someItem = someItems.Cast<Node>().First();
-
-        Func<Node, Pair<Node, string>> keyExtractor =
-          n => new Pair<Node, string>(n, GetNodeComparisonKey(n));
-
-        var sourceKeyMap = src
-          .Cast<Node>()
-          .Select(keyExtractor)
-          .ToDictionary(pair => pair.Second, pair => pair.First, StringComparer.OrdinalIgnoreCase);
-        var targetKeyMap = tgt
-          .Cast<Node>()
-          .Select(keyExtractor)
-          .ToDictionary(pair => pair.Second, pair => pair.First, StringComparer.OrdinalIgnoreCase);
-
-        var sourceKeys = src.Cast<Node>().Select(n => keyExtractor(n).Second);
-        var targetKeys = tgt.Cast<Node>().Select(n => keyExtractor(n).Second);
-        var commonKeys = sourceKeys.Intersect(targetKeys, StringComparer.OrdinalIgnoreCase);
-
-        var sequence =
-          sourceKeys.Except(commonKeys, StringComparer.OrdinalIgnoreCase)
-            .Select(k => new {Index = sourceKeyMap[k].Index, Type = 0,
-              Source = sourceKeyMap[k], Target = (Node) null})
-          .Concat(commonKeys
-            .Select(k => new {Index = targetKeyMap[k].Index, Type = 1,
-              Source = sourceKeyMap[k], Target = targetKeyMap[k]}))
-          .Concat(targetKeys.Except(commonKeys, StringComparer.OrdinalIgnoreCase)
-            .Select(k => new {Index = targetKeyMap[k].Index, Type = 2,
-              Source = (Node) null, Target = targetKeyMap[k]}))
-          .OrderBy(i => i.Type!=0).ThenBy(i => i.Index).ThenBy(i => i.Type);
-
-        foreach (var i in sequence) {
-          var d = Visit(i.Source, i.Target);
-          if (d!=null)
+        foreach (var targetItem in targetKeyMap) {
+          var (s, t) = sourceKeyMap.ContainsKey(targetItem.Key)
+            ? (sourceKeyMap[targetItem.Key], targetKeyMap[targetItem.Key])
+            : (null, targetKeyMap[targetItem.Key]);
+          var d = Visit(s, t);
+          if (d != null) {
             difference.ItemChanges.Add((NodeDifference) d);
+          }
+          
         }
+        difference.ItemChanges.Sort(CompareNodeDifference);
 
-        return (difference.ItemChanges.Count!=0) ? difference : null;
+        return difference.ItemChanges.Count != 0 ? difference : null;
       }
     }
+    
+    // Sort by items only with source, then by (target ?? source).Index then with source and target and then only with target
+    private static int CompareNodeDifference(NodeDifference curr, NodeDifference other)
+    {
+      var currType = curr.Source != null && curr.Target != null ? 1 : curr.Source == null ? 3 : 0; 
+      var otherType = other.Source != null && other.Target != null ? 1 : other.Source == null ? 3 : 0; 
+      var typeIsNot0Comparison = (currType != 0).CompareTo(otherType != 0);
+      if (typeIsNot0Comparison != 0) {
+        return typeIsNot0Comparison;
+      }
+
+      var currIndex = (curr.Target ?? curr.Source)?.Index ?? 0;
+      var otherIndex = (other.Target ?? other.Source)?.Index ?? 0;
+      var indexComparison = currIndex.CompareTo(otherIndex);
+      return indexComparison != 0 ? indexComparison : currType.CompareTo(otherType);
+    }
 
     /// <summary>
     /// Visits specified objects.

Orm/Xtensive.Orm/Modelling/Comparison/Upgrader.cs

@@ -121,7 +121,7 @@ public ReadOnlyList<NodeAction> GetUpgradeSequence(Difference difference, HintSe
       ArgumentValidator.EnsureArgumentNotNull(hints, nameof(hints));
       ArgumentValidator.EnsureArgumentNotNull(comparer, nameof(comparer));
       if (difference == null) {
-        return new ReadOnlyList<NodeAction>(Enumerable.Empty<NodeAction>().ToList());
+        return ReadOnlyList<NodeAction>.Empty;
       }
 
       TemporaryRenames = new Dictionary<string, Node>(StringComparer.OrdinalIgnoreCase);

Orm/Xtensive.Orm/Modelling/Node.cs

@@ -34,6 +34,7 @@ public abstract class Node : LockableBase,
     /// Path delimiter character.
     /// </summary>
     public static readonly char PathDelimiter = '/';
+    public static readonly string PathDelimiterString = PathDelimiter.ToString();
     /// <summary>
     /// Path escape character.
     /// </summary>
@@ -185,18 +186,24 @@ public IEnumerable<Pair<string, IPathNode>> GetPathNodes(bool nestedOnly)
     public string Path {
       [DebuggerStepThrough]
       get {
-        if (cachedPath!=null)
+        if (cachedPath!=null) {
           return cachedPath;
-        if (Parent==null)
+        }
+
+        if (Parent==null) {
           return string.Empty;
-        string parentPath = Parent.Path;
-        if (parentPath.Length!=0)
-          parentPath += PathDelimiter;
+        }
+
+        var parentPath = Parent.Path;
+        if (parentPath.Length!=0) {
+          parentPath += PathDelimiterString;
+        }
+
         return string.Concat(
           parentPath,
           Nesting.EscapedPropertyName,
-          Nesting.IsNestedToCollection ? PathDelimiter.ToString() : string.Empty,
-          Nesting.IsNestedToCollection ? EscapedName : string.Empty);
+          Nesting.IsNestedToCollection ? PathDelimiterString : null,
+          Nesting.IsNestedToCollection ? EscapedName : null);
       }
     }
 
@@ -972,7 +979,7 @@ public override string ToString()
       var m = Model;
       var fullName = Path;
       if (m != null) {
-        fullName = string.Concat(m.EscapedName, PathDelimiter, fullName);
+        fullName = string.Concat(m.EscapedName, PathDelimiterString, fullName);
       }
 
       if (!Nesting.IsNestedToCollection && !(this is IModel)) {

Orm/Xtensive.Orm/Modelling/NodeCollection.cs

@@ -5,18 +5,13 @@
 // Created:    2009.03.16
 
 using System;
-using System.Linq;
 using System.Collections;
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Diagnostics;
 using System.Runtime.Serialization;
-using Xtensive.Collections;
 using Xtensive.Core;
 
-using Xtensive.Modelling;
-using Xtensive.Modelling.Comparison;
-
 
 namespace Xtensive.Modelling
 {
@@ -28,8 +23,6 @@ public abstract class NodeCollection : LockableBase,
     INodeCollection,
     IDeserializationCallback
   {
-    private static readonly ReadOnlyList<Node> emptyCountable =
-      new ReadOnlyList<Node>(new List<Node>(), false);
     [NonSerialized]
     private string escapedName;
     [NonSerialized]
@@ -90,7 +83,7 @@ public string Path {
         if (parentPath.Length==0)
           return EscapedName;
         return string.Concat(
-          parentPath, Node.PathDelimiter, 
+          parentPath, Node.PathDelimiterString,
           EscapedName);
       }
     }
@@ -355,7 +348,7 @@ public override string ToString()
       var m = Model;
       string fullName = Path;
       if (m!=null)
-        fullName = string.Concat(m.EscapedName, Node.PathDelimiter, fullName);
+        fullName = string.Concat(m.EscapedName, Node.PathDelimiterString, fullName);
       return string.Format(Strings.NodeInfoFormat, fullName, Count);
     }
 

Orm/Xtensive.Orm/Orm/Building/PrefetchActionContainer.cs

@@ -42,7 +42,7 @@ public PrefetchActionContainer(TypeInfo type, List<AssociationInfo> associations
     {
       this.type = type;
       this.associations = associations;
-      fields = new ReadOnlyList<PrefetchFieldDescriptor>(new List<PrefetchFieldDescriptor>());
+      fields = ReadOnlyList<PrefetchFieldDescriptor>.Empty;
     }
   }
 }

Orm/Xtensive.Orm/Orm/Model/TypeInfo.cs

@@ -600,10 +600,12 @@ public override void UpdateState()
       targetAssociations = new ReadOnlyList<AssociationInfo>(GetTargetAssociations());
       ownerAssociations = new ReadOnlyList<AssociationInfo>(GetOwnerAssociations());
 
-      int adapterIndex = 0;
-      foreach (FieldInfo field in Fields)
-        if (field.IsStructure || field.IsEntitySet)
+      var adapterIndex = 0;
+      foreach (var field in Fields) {
+        if (field.IsStructure || field.IsEntitySet) {
           field.AdapterIndex = adapterIndex++;
+        }
+      }
 
       affectedIndexes.UpdateState();
       indexes.UpdateState();
@@ -618,14 +620,14 @@ public override void UpdateState()
 
       if (IsEntity) {
         if (HasVersionRoots) {
-          versionFields = new ReadOnlyList<FieldInfo>(new List<FieldInfo>());
+          versionFields = ReadOnlyList<FieldInfo>.Empty;
           versionColumns = new ReadOnlyList<ColumnInfo>(new List<ColumnInfo>());
         }
         else {
           versionFields = new ReadOnlyList<FieldInfo>(GetVersionFields());
           versionColumns = new ReadOnlyList<ColumnInfo>(GetVersionColumns());
         }
-        HasVersionFields = versionFields.Any();
+        HasVersionFields = versionFields.Count > 0;
         HasExplicitVersionFields = versionFields.Any(f => f.ManualVersion || f.AutoVersion);
       }
 
@@ -634,7 +636,7 @@ public override void UpdateState()
         // We'll check that all implementors are mapped to the same database later.
         // MappingSchema is not important: it's copied for consistency.
         var firstImplementor = GetImplementors().FirstOrDefault();
-        if (firstImplementor!=null) {
+        if (firstImplementor != null) {
           MappingDatabase = firstImplementor.MappingDatabase;
           MappingSchema = firstImplementor.MappingSchema;
         }
@@ -698,10 +700,10 @@ public override void UpdateState()
       sequence.AddRange(b);
 
       var first = sequence.Where(a => a.Ancestors.Count > 0).ToList();
-      if (first.Count==0)
+      if (first.Count == 0)
         removalSequence = new ReadOnlyList<AssociationInfo>(sequence);
       else {
-        var second = sequence.Where(a => a.Ancestors.Count==0).ToList();
+        var second = sequence.Where(a => a.Ancestors.Count == 0);
         removalSequence = new ReadOnlyList<AssociationInfo>(first.Concat(second).ToList());
       }
     }
@@ -750,7 +752,7 @@ private bool GetIsLeaf()
 
     private void CreateTupleDescriptor()
     {
-      var orderedColumns = columns.OrderBy(c => c.Field.MappingInfo.Offset).ToList();
+      var orderedColumns = columns.OrderBy(c => c.Field.MappingInfo.Offset);
       columns = new ColumnInfoCollection(this, "Columns");
       columns.AddRange(orderedColumns);
       TupleDescriptor = TupleDescriptor.Create(

Orm/Xtensive.Orm/Orm/Operation.cs

@@ -25,8 +25,7 @@ public abstract class Operation : IOperation
   {
     private static readonly ReadOnlyDictionary<string, Key> EmptyIdentifiedEntities = 
       new ReadOnlyDictionary<string, Key>(new Dictionary<string, Key>());
-    private static readonly ReadOnlyList<IOperation> EmptyOperations = 
-      new ReadOnlyList<IOperation>(new List<IOperation>());
+    private static readonly ReadOnlyList<IOperation> EmptyOperations = ReadOnlyList<IOperation>.Empty;
 
     private ReadOnlyDictionary<string, Key> identifiedEntities = EmptyIdentifiedEntities;
     private ReadOnlyList<IOperation> precedingOperations = EmptyOperations;

Orm/Xtensive.Orm/Orm/Upgrade/Hints/ChangeFieldTypeHint.cs

@@ -88,7 +88,7 @@ public ChangeFieldTypeHint(Type type, string fieldName)
 
       Type = type;
       FieldName = fieldName;
-      AffectedColumns = new ReadOnlyList<string>(new List<string>());
+      AffectedColumns = ReadOnlyList<string>.Empty;
     }
 
     /// <summary>

Orm/Xtensive.Orm/Orm/Upgrade/Hints/RemoveFieldHint.cs

@@ -91,7 +91,7 @@ public RemoveFieldHint(string type, string field)
       ArgumentValidator.EnsureArgumentNotNullOrEmpty(field, "field");
       Type = type;
       Field = field;
-      AffectedColumns = new ReadOnlyList<string>(new List<string>());
+      AffectedColumns = ReadOnlyList<string>.Empty;
     }
 
     /// <summary>
@@ -106,7 +106,7 @@ public RemoveFieldHint(Type type, string field)
 
       Type = type.FullName;
       Field = field;
-      AffectedColumns = new ReadOnlyList<string>(new List<string>());
+      AffectedColumns = ReadOnlyList<string>.Empty;
     }
 
     /// <summary>

Orm/Xtensive.Orm/Orm/Upgrade/Hints/RemoveTypeHint.cs

@@ -75,7 +75,7 @@ public RemoveTypeHint(string type)
     {
       ArgumentValidator.EnsureArgumentNotNullOrEmpty(type, "sourceType");
       Type = type;
-      AffectedTables = new ReadOnlyList<string>(new List<string>());
+      AffectedTables = ReadOnlyList<string>.Empty;
     }
   }
 }