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.

Sign up for GitHub

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

Skip lambda allocation in FindTransientAnnotation #2154

Merged
merged 6 commits into from
Aug 13, 2021
Merged

Skip lambda allocation in FindTransientAnnotation #2154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50e78b4
Skip lambda allocation in FindTransientAnnotation
joaocpaiva Aug 6, 2021
1ae1e14
Removing LinkedVersioningList enumerator allocation cost
joaocpaiva Aug 6, 2021
fa9dab7
Fix versioning list tests
joaocpaiva Aug 7, 2021
6cc094a
Remove annotationsList variable
joaocpaiva Aug 9, 2021
d139d44
Remove var usage
joaocpaiva Aug 9, 2021
2f1fd4a
Remove product-specific references from comments
joaocpaiva Aug 9, 2021
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
1 change: 0 additions & 1 deletion src/Microsoft.OData.Edm/GlobalSuppressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1710:IdentifiersShouldHaveCorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningList`1")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1710:IdentifiersShouldHaveCorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningList`1+ArrayVersioningList")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1710:IdentifiersShouldHaveCorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningList`1+EmptyVersioningList")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1710:IdentifiersShouldHaveCorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningList`1+LinkedVersioningList")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningDictionary`2")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningDictionary`2+EmptyVersioningDictionary")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1711:IdentifiersShouldNotHaveIncorrectSuffix", Scope = "type", Target = "Microsoft.OData.Edm.VersioningDictionary`2+HashTreeDictionary")]
Expand Down
147 changes: 6 additions & 141 deletions src/Microsoft.OData.Edm/VersioningList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.OData.Edm
/// "Mutating" operations return a new list (which, for efficiency, may share some of the state of the old one).
/// </summary>
/// <typeparam name="TElement">Element type of the list.</typeparam>
internal abstract class VersioningList<TElement> : IEnumerable<TElement>
internal abstract class VersioningList<TElement> : IReadOnlyList<TElement>, IEnumerable<TElement>
{
public abstract int Count { get; }

Expand Down Expand Up @@ -68,7 +68,7 @@ public override int Count

public override VersioningList<TElement> Add(TElement value)
{
return new LinkedVersioningList(this, value);
return new ArrayVersioningList(this, value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the implementation, this results in a new array being allocated, and elements copied from the previous array to the new one. Doesn't this lead to a larger memory footprint and more GC over time? Is the rationale that we're willing to sacrifice memory in order to get faster search times?

}

public override IEnumerator<TElement> GetEnumerator()
Expand Down Expand Up @@ -113,154 +113,19 @@ public void Reset()
}
}

internal sealed class LinkedVersioningList : VersioningList<TElement>
{
private readonly VersioningList<TElement> preceding;
private readonly TElement last;

public LinkedVersioningList(VersioningList<TElement> preceding, TElement last)
{
this.preceding = preceding;
this.last = last;
}

public override int Count
{
get { return this.preceding.Count + 1; }
}

public VersioningList<TElement> Preceding
{
get { return this.preceding; }
}

public TElement Last
{
get { return this.last; }
}

private int Depth
{
get
{
int depth = 0;
LinkedVersioningList layer = this;
while (layer != null)
{
depth++;
layer = layer.Preceding as LinkedVersioningList;
}

return depth;
}
}

public override VersioningList<TElement> Add(TElement value)
{
if (this.Depth < 5)
{
return new LinkedVersioningList(this, value);
}

return new ArrayVersioningList(this, value);
}

public override IEnumerator<TElement> GetEnumerator()
{
return new LinkedListEnumerator(this);
}

protected override TElement IndexedElement(int index)
{
if (index == this.Count - 1)
{
return this.last;
}

return this.preceding.IndexedElement(index);
}

protected override VersioningList<TElement> RemoveIndexedElement(int index)
{
if (index == this.Count - 1)
{
return this.preceding;
}

return new LinkedVersioningList(this.preceding.RemoveIndexedElement(index), this.last);
}
}

internal sealed class LinkedListEnumerator : IEnumerator<TElement>
{
private readonly LinkedVersioningList list;
private IEnumerator<TElement> preceding;
private bool complete;

public LinkedListEnumerator(LinkedVersioningList list)
{
this.list = list;
this.preceding = list.Preceding.GetEnumerator();
}

public TElement Current
{
get
{
if (this.complete)
{
return this.list.Last;
}

return this.preceding.Current;
}
}

object System.Collections.IEnumerator.Current
{
get { return this.Current; }
}

public void Dispose()
{
}

public bool MoveNext()
{
if (this.complete)
{
return false;
}

if (!this.preceding.MoveNext())
{
this.complete = true;
}

return true;
}

public void Reset()
{
this.preceding.Reset();
this.complete = false;
}
}

internal sealed class ArrayVersioningList : VersioningList<TElement>
{
private readonly TElement[] elements;

public ArrayVersioningList(VersioningList<TElement> preceding, TElement last)
{
this.elements = new TElement[preceding.Count + 1];
int index = 0;
foreach (TElement element in preceding)
for (int i = 0; i < preceding.Count; i++)
{
this.elements[index++] = element;
this.elements[i] = preceding[i];
}

this.elements[index] = last;
this.elements[preceding.Count] = last;
}

private ArrayVersioningList(TElement[] elements)
Expand All @@ -280,7 +145,7 @@ public TElement ElementAt(int index)

public override VersioningList<TElement> Add(TElement value)
{
return new LinkedVersioningList(this, value);
return new ArrayVersioningList(this, value);
}

public override IEnumerator<TElement> GetEnumerator()
Expand Down
Loading