Various performance improvements (#488)

Remove unnecessary calls to *.Contains()
* Combine .!Contains with .Add into a single Add operation
* Replace IndexOf with Contains (faster remove from EntityCollection)

Go though usages of ContainsKey
* Use TryAdd instead if followed by Add
* Use TryGet if followed by fetching value
* Switch to HashSet if value is not used

Replace unneccesary IndexOf calls
* When access EntitySet of EntityCollection using CollectioViewSource add and Contains checks are much faster O(1) instead of O(n)

This translates to roughly ~9% perf improvement for adding 50 000 entities to an EntityCollection.

src/OpenRiaServices.Client.DomainClients.Http/Framework/SystemServiceModel.Dispatcher/QueryStringConverter.cs

@@ -159,9 +159,9 @@ public virtual string ConvertValueToString(object parameter, Type parameterType)
         [SuppressMessage("Reliability", "Reliability104:CaughtAndHandledExceptionsRule", Justification = "The exception is traced in the finally clause")]
         TypeConverter GetStringConverter(Type parameterType)
         {
-            if (this._typeConverterCache.ContainsKey(parameterType))
+            if (this._typeConverterCache.TryGetValue(parameterType, out TypeConverter typeConverter))
             {
-                return (TypeConverter)this._typeConverterCache[parameterType];
+                return typeConverter;
             }
             TypeConverterAttribute[] typeConverterAttrs = parameterType.GetCustomAttributes(typeof(TypeConverterAttribute), true) as TypeConverterAttribute[];
             if (typeConverterAttrs != null)

src/OpenRiaServices.Client/Framework/ChangeSetBuilder.cs

@@ -149,7 +149,7 @@ internal static void CheckForInvalidUpdates(EntityChangeSet changeSet)
         /// </summary>
         internal class UnmodifiedOperationAdder : EntityVisitor
         {
-            private readonly Dictionary<object, bool> _visited = new Dictionary<object, bool>();
+            private readonly HashSet<object> _visited = new HashSet<object>();
             private readonly List<ChangeSetEntry> _changeSetEntries;
             private int _id;
             private bool _isChild;
@@ -180,7 +180,7 @@ public static void Add(List<ChangeSetEntry> changeSetEntries)
 
             public override void Visit(Entity entity)
             {
-                if (this._visited.ContainsKey(entity))
+                if (!this._visited.Add(entity))
                 {
                     return;
                 }
@@ -192,8 +192,6 @@ public override void Visit(Entity entity)
                     this._changeSetEntries.Add(op);
                 }
 
-                this._visited.Add(entity, true);
-
                 base.Visit(entity);
             }
 

src/OpenRiaServices.Client/Framework/EntityCollection.cs

@@ -259,9 +259,8 @@ public void Add(TEntity entity)
 
                 // we may have to check for containment once more, since the EntitySet.Add calls
                 // above can cause a dynamic add to this EntityCollection behind the scenes
-                if (!addedToSet || !this.EntitiesHashSet.Contains(entity))
+                if (TryAddEntity(entity) || addedToSet)
                 {
-                    this.AddEntity(entity);
                     this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Add, entity, this.Entities.Count - 1);
                 }
 
@@ -350,28 +349,34 @@ public override string ToString()
         /// should be done through this method.
         /// </summary>
         /// <param name="entity">The <see cref="Entity"/>to add.</param>
-        private void AddEntity(TEntity entity)
+        private bool TryAddEntity(TEntity entity)
         {
-            Debug.Assert(!this.EntitiesHashSet.Contains(entity), "Entity is already in this collection!");
+            if (this.EntitiesHashSet.Add(entity))
+            {
+                this.Entities.Add(entity);
 
-            this.Entities.Add(entity);
-            this.EntitiesHashSet.Add(entity);
+                if (this.IsComposition)
+                {
+                    entity.SetParent(this._parent, this.AssocAttribute);
+                }
 
-            if (this.IsComposition)
+                return true;
+            }
+            else
             {
-                entity.SetParent(this._parent, this.AssocAttribute);
+                return false;
             }
         }
 
         private bool RemoveEntity(TEntity entity)
         {
-            var isRemoved = this.Entities.Remove(entity);
-            var isRemovedInHashSet = this.EntitiesHashSet.Remove(entity);
-            Debug.Assert(isRemoved == isRemovedInHashSet
-                , "The entity should be present in both Entities and EntitiesHashSet"
-                , "Entities.Removed: {0}, EntitiesHashSet.Removed: {1}", isRemoved, isRemovedInHashSet
-            );
-            return isRemoved;
+            if (this.EntitiesHashSet.Remove(entity))
+            {
+                bool isRemoved = this.Entities.Remove(entity);
+                Debug.Assert(isRemoved, "The entity should be present in both Entities and EntitiesHashSet");
+                return true;
+            }
+            return false;
         }
 
         /// <summary>
@@ -431,10 +436,7 @@ private void Load()
             EntitySet set = this._parent.EntitySet.EntityContainer.GetEntitySet(typeof(TEntity));
             foreach (TEntity entity in set.OfType<TEntity>().Where(this.Filter))
             {
-                if (!this.EntitiesHashSet.Contains(entity))
-                {
-                    this.AddEntity(entity);
-                }
+                this.TryAddEntity(entity);
             }
 
             // once we've loaded entities, we're caching them, so we need to update
@@ -618,7 +620,8 @@ private void OnEntityAssociationUpdated(Entity entity)
                 {
                     // Add matching entity to our set. When adding, we use the stronger Filter to
                     // filter out New entities
-                    this.AddEntity(typedEntity);
+                    bool added = this.TryAddEntity(typedEntity);
+                    Debug.Assert(added);
                     this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Add, typedEntity, this.Entities.Count - 1);
                 }
                 else if (containsEntity && !this._entityPredicate(typedEntity))
@@ -653,9 +656,8 @@ private void SourceSet_CollectionChanged(object sender, NotifyCollectionChangedE
                     foreach (TEntity newEntity in newEntities)
                     {
                         newStartingIdx = this.Entities.Count;
-                        if (!this.EntitiesHashSet.Contains(newEntity))
+                        if (this.TryAddEntity(newEntity))
                         {
-                            this.AddEntity(newEntity);
                             affectedEntities.Add(newEntity);
                         }
                     }
@@ -825,7 +827,7 @@ ICollectionView ICollectionViewFactory.CreateView()
         /// is sufficient for interaction with the ListCollectionView.
         /// </remarks>
         /// <typeparam name="T">The entity type of this proxy</typeparam>
-        private class ListCollectionViewProxy<T> : IList, IEnumerable<T>, INotifyCollectionChanged, ICollectionChangedListener where T : Entity
+        internal class ListCollectionViewProxy<T> : IList, IEnumerable<T>, INotifyCollectionChanged, ICollectionChangedListener where T : Entity
         {
             private readonly object _syncRoot = new object();
             private readonly EntityCollection<T> _source;
@@ -855,8 +857,10 @@ public int Add(object value)
                 }
 
                 this._addedEntities.Add(entity);
+                int countBefore = this.Source.Count;
                 this.Source.Add(entity);
-                return this.IndexOf(entity);
+
+                return this.Source.Entities.IndexOf(entity, countBefore);
             }
 
             public void Clear()
@@ -867,7 +871,7 @@ public void Clear()
 
             public bool Contains(object value)
             {
-                return this.IndexOf(value) >= 0;
+                return this.Source.EntitiesHashSet.Contains(value);
             }
 
             public int IndexOf(object value)
@@ -1029,9 +1033,9 @@ bool ICollection<TEntity>.Contains(TEntity item)
         }
         bool ICollection<TEntity>.Remove(TEntity item)
         {
-            int idx = Entities.IndexOf(item);
+            bool removed = this.EntitiesHashSet.Contains(item);
             Remove(item);
-            return idx != -1;
+            return removed;
         }
         /// <summary>
         /// Removes all items.

src/OpenRiaServices.Client/Framework/EntityContainer.cs

@@ -93,16 +93,15 @@ public void AddReference(EntitySet entitySet)
             }
 
             Type entityType = entitySet.EntityType;
-            if (this._entitySets.ContainsKey(entityType) || this._referencedEntitySets.ContainsKey(entityType))
+            if (this._entitySets.ContainsKey(entityType) 
+                || !this._referencedEntitySets.TryAdd(entityType, entitySet))
             {
                 throw new ArgumentException(
                     string.Format(
                         CultureInfo.InvariantCulture,
                         Resource.EntityContainer_EntitySetAlreadyExists,
                         entityType));
             }
-
-            this._referencedEntitySets.Add(entityType, entitySet);
         }
 
         /// <summary>

src/OpenRiaServices.Client/Framework/EntitySet.cs

@@ -730,7 +730,7 @@ internal Entity LoadEntity(Entity entity, LoadBehavior loadBehavior)
             if (cachedEntity == null)
             {
                 // add the entity to the cache
-                this.AddToCache(entity);
+                this._identityCache.Add(identity, entity);
                 cachedEntity = entity;
 
                 int idx = 0;
@@ -1425,10 +1425,10 @@ public int Add(object value)
 
                 int countBefore = this.Source.Count;
                 this.Source.Add(entity);
-                int countAfter = this.Source.Count;
 
-                // If count increased then the item was added last, otherwise return -1 for "not added"
-                return (countAfter > countBefore) ? countBefore : -1;
+                return this.Source.Count == countBefore + 1 
+                    ? countBefore
+                    : ((List<T>)this.Source.List).IndexOf(entity, countBefore);
             }
 
             public void Clear()

src/OpenRiaServices.Client/Framework/ObjectStateUtility.cs

@@ -28,14 +28,10 @@ private static IDictionary<string, object> ExtractState(object o, HashSet<object
             MetaType metaType = MetaType.GetMetaType(o.GetType());
             Dictionary<string, object> extractedState = new Dictionary<string, object>();
 
-            if (visited.Contains(o))
+            if (!visited.Add(o))
             {
                 throw new InvalidOperationException(Resource.CyclicReferenceError);
             }
-            else
-            {
-                visited.Add(o);
-            }
 
             foreach (MetaMember metaMember in metaType.DataMembers)
             {

src/OpenRiaServices.Client/Framework/Polyfills.cs

@@ -1,9 +1,8 @@
-using System;
-using System.Collections.Generic;
+#if !NET
+using System;
 
-namespace OpenRiaServices.Client
+namespace System.Collections.Generic
 {
-#if !NET
     /// <summary>
     /// Helper methods to allow "newer" .NET methods on older frameworks
     /// </summary>
@@ -25,5 +24,5 @@ public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionar
             }
         }
     }
-#endif
 }
+#endif

src/OpenRiaServices.Client/Test/Client.Test/Data/EntityCollectionTests.cs

@@ -49,6 +49,28 @@ public void ICVF_AddNew()
                 "EntitySet should contain the first entity after CommitNew.");
         }
 
+        [TestMethod]
+        [Description("Tests that ListCollectionViewProxy returns correct index")]
+        public void ICVF_ListCollectionViewProxy_Add()
+        {
+            EntitySet<City> entitySet;
+            EntityCollection<City> entityCollection = this.CreateEntityCollection(out entitySet);
+            EntityCollection<City>.ListCollectionViewProxy<City> collection = new(entityCollection);
+
+            for (int i=0; i < 3; ++i)
+            {
+                var city = new City() {  ZoneID = i };
+
+                Assert.IsFalse(collection.Contains(city));
+                int idx = collection.Add(city);
+
+                Assert.AreEqual(i, idx);
+                Assert.AreSame(city, collection[idx]);
+                Assert.IsTrue(collection.Contains(city));
+                Assert.IsTrue(entityCollection.Contains(city));
+            }
+        }
+
         [TestMethod]
         [Description("Tests that calling AddNew on the View adds to the EntityCollection and EntitySet and CancelNew removes both.")]
         public void ICVF_CancelNew()

src/OpenRiaServices.Hosting.Wcf/Framework/Linq/DynamicQueryable.cs

@@ -359,9 +359,8 @@ void ProcessParameters(ParameterExpression[] parameters)
 
         void AddSymbol(string name, object value)
         {
-            if (symbols.ContainsKey(name))
+            if (!symbols.TryAdd(name, value))
                 throw ParseError(Resource.DuplicateIdentifier, name);
-            symbols.Add(name, value);
         }
 
         public Expression Parse(Type resultType)

src/OpenRiaServices.Hosting.Wcf/Framework/Wcf/ChangeSetProcessor.cs

@@ -208,11 +208,10 @@ private static void SetEntityAssociations(IEnumerable<ChangeSetEntry> changeSetE
 #pragma warning restore CS0618 // Type or member is obsolete
                 foreach (ChangeSetEntry changeSetEntry in entityGroup)
                 {
-                    if (visited.Contains(changeSetEntry.Id))
+                    if (!visited.Add(changeSetEntry.Id))
                     {
                         continue;
                     }
-                    visited.Add(changeSetEntry.Id);
 
                     // set current associations
                     if (changeSetEntry.Associations != null)