Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection performance improvements #488

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions src/OpenRiaServices.Client/Framework/ChangeSetBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -192,8 +192,6 @@ public override void Visit(Entity entity)
this._changeSetEntries.Add(op);
}

this._visited.Add(entity, true);

base.Visit(entity);
}

Expand Down
58 changes: 31 additions & 27 deletions src/OpenRiaServices.Client/Framework/EntityCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions src/OpenRiaServices.Client/Framework/EntityContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions src/OpenRiaServices.Client/Framework/EntitySet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@
if (cachedEntity == null)
{
// add the entity to the cache
this.AddToCache(entity);
this._identityCache.Add(identity, entity);
cachedEntity = entity;

int idx = 0;
Expand Down Expand Up @@ -1425,10 +1425,10 @@

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);
Dismissed Show dismissed Hide dismissed
}

public void Clear()
Expand Down
6 changes: 1 addition & 5 deletions src/OpenRiaServices.Client/Framework/ObjectStateUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
9 changes: 4 additions & 5 deletions src/OpenRiaServices.Client/Framework/Polyfills.cs
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -25,5 +24,5 @@ public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionar
}
}
}
#endif
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading