Skip to content

Commit

Permalink
Improve Add/Attach performance for large graphs (#487)
Browse files Browse the repository at this point in the history 
Makes adding n entities to an EntityCollection or similar roughly O(n) instead of O(n²)

* Don't further process entities which are New, Unchanged, Modified since any changes to them would have already been discovered
* Calculate and cache AssociationMembers once
* Do some code/style cleanup which might also give marginally better perf (calls contains on HashSet instead of Enumerable) and smarter enum checks due to pattern matchning
* Always package nuget's so that PR artifacts can be tested
* minor EntitySet perf improvements
   * Use result of HashSet.Add instead of checking Contains before Add
   * Make ListCollectionViewProxy smarter
     * remove IndexOf from Add for O(1) instead of O(n) performance
     * Contains use hashSet contains instead of IndeOf for O(1) instead of O(n) performance
     * Remove argument range check
* EntitySet Make _set and _identityCache fields readonly
  • Loading branch information
@Daniel-Svensson
Daniel-Svensson authored Feb 7, 2024
1 parent 0f00bbd commit 069a03f
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 141 deletions.
2 changes: 1 addition & 1 deletion azure-pipelines.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ steps:
versionEnvVar: Build.BuildNumber
packDestination: '$(Build.ArtifactStagingDirectory)'
continueOnError: true
condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))
# condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest'))

- task: CopyFiles@2
displayName: 'Copy VS Extension: $(build.artifactstagingdirectory)'
Expand Down
64 changes: 19 additions & 45 deletions src/OpenRiaServices.Client/Framework/Entity.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ private set
EntitySet entitySet = this.LastSet;
if (entitySet != null)
{
bool isInteresting = this._entityState != EntityState.Unmodified
&& this._entityState != EntityState.Detached;
bool isInteresting = this._entityState is not EntityState.Unmodified
and not EntityState.Detached;
entitySet.TrackAsInteresting(this, isInteresting);
}

Expand Down Expand Up @@ -302,12 +302,9 @@ internal void OnChildUpdate()
}
}

if (this.EntitySet != null)
{
// when a child has been changed in any way, the parent becomes
// interesting
this.EntitySet.TrackAsInteresting(this, true);
}
// when a child has been changed in any way, the parent becomes
// interesting
this.EntitySet?.TrackAsInteresting(this, true);
}

/// <summary>
Expand Down Expand Up @@ -526,8 +523,7 @@ private set
this._isMerging = value;
foreach (MetaMember metaMember in MetaType.DataMembers.Where(f => f.IsComplex && !f.IsCollection))
{
ComplexObject propertyValue = metaMember.GetValue(this) as ComplexObject;
if (propertyValue != null)
if (metaMember.GetValue(this) is ComplexObject propertyValue)
{
propertyValue.IsMergingState = this._isMerging;
}
Expand Down Expand Up @@ -647,10 +643,7 @@ internal void Reset()
this._editSession = null;
this._trackChanges = false;
UndoAllEntityActions(throwIfSubmitting: true);
if (this._validationErrors != null)
{
this._validationErrors.Clear();
}
this._validationErrors?.Clear();
this.EntityConflict = null;
this.EntitySet = null;
this._lastSet = null;
Expand Down Expand Up @@ -682,8 +675,8 @@ internal void StopTracking()
/// </summary>
protected void AcceptChanges()
{
if (this.EntityState == EntityState.Unmodified ||
this.EntityState == EntityState.Detached)
if (this.EntityState is EntityState.Unmodified or
EntityState.Detached)
{
// if we're detached or have no changes, noop after
// closing any in progress edit session
Expand All @@ -696,11 +689,7 @@ protected void AcceptChanges()
// Accept any child changes. Note, we must accept child changes before setting our own
// state to Unmodified. This avoids a situation where we get notifications from child
// entities that cause our state to flip back to Modified.
if (entitySet != null)
{
// accept any child changes
entitySet.EntityContainer.CompleteChildChanges(this, true);
}
entitySet?.EntityContainer.CompleteChildChanges(this, true);

if (this.EntityState == EntityState.New)
{
Expand Down Expand Up @@ -730,30 +719,21 @@ protected void AcceptChanges()
else if (this.EntityState == EntityState.Deleted)
{
this.StopTracking();
if (entitySet != null)
{
entitySet.RemoveFromCache(this);
}
entitySet?.RemoveFromCache(this);
// move back to the default state
this.EntityState = EntityState.Detached;
}

if (entitySet != null)
{
// remove from the interesting entities set
entitySet.TrackAsInteresting(this, false);
}
// remove from the interesting entities set
entitySet?.TrackAsInteresting(this, false);

// need to end any in progress edit session
this._editSession = null;

// clear all custom method invocations
this.UndoAllEntityActions(throwIfSubmitting: false);

if (this._validationErrors != null)
{
this._validationErrors.Clear();
}
this._validationErrors?.Clear();
this.EntityConflict = null;
this.IsInferred = false;
this._hasChildChanges = false;
Expand All @@ -769,8 +749,8 @@ protected void AcceptChanges()
/// </summary>
protected void RejectChanges()
{
if (this.EntityState == EntityState.Unmodified ||
this.EntityState == EntityState.Detached)
if (this.EntityState is EntityState.Unmodified or
EntityState.Detached)
{
// if we're detached or have no changes, noop after
// closing any in progress edit session
Expand All @@ -783,10 +763,7 @@ protected void RejectChanges()
// Reject any child changes. Note, we must reject child changes before setting our own
// state to Unmodified. This avoids a situation where we get notifications from child
// entities that cause our state to flip back to Modified.
if (entitySet != null)
{
entitySet.EntityContainer.CompleteChildChanges(this, false);
}
entitySet?.EntityContainer.CompleteChildChanges(this, false);

if (this._entityState == EntityState.Modified || this.Parent != null)
{
Expand Down Expand Up @@ -830,10 +807,7 @@ protected void RejectChanges()
UndoAllEntityActions();

// Empty out the error collections
if (this._validationErrors != null)
{
this._validationErrors.Clear();
}
this._validationErrors?.Clear();
this.EntityConflict = null;
this._hasChildChanges = false;
Debug.Assert(!this.HasChanges, "Entity.HasChanges should be false");
Expand Down Expand Up @@ -1845,7 +1819,7 @@ private EditSession(Entity entity)
{
this._entity = entity;
this._lastState = entity.EntityState;
this._customMethodInvocations = entity._customMethodInvocations != null ? entity._customMethodInvocations.ToArray() : null;
this._customMethodInvocations = entity._customMethodInvocations?.ToArray();
this._validationErrors = entity.ValidationErrors.ToArray();
this._modifiedProperties = new List<string>();
}
Expand Down
Loading

0 comments on commit 069a03f

Please sign in to comment.