Skip to content

Commit

Permalink
Improve the performance of ImmutableList.Contains (dotnet/corefx#40540)
Browse files Browse the repository at this point in the history
* Improve ImmutableList.IndexOf performance

* Implement a spicific Contains method

IndexOf has a lot of overhead because it's looking for an index when
all Contains wants to do is find a matching element.

* Fix tests

* Revert change to IndexOf


Commit migrated from dotnet/corefx@a866d96
  • Loading branch information
shortspider authored and stephentoub committed Oct 19, 2019
1 parent ddfb559 commit fa9a944
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,22 @@ internal int BinarySearch(int index, int count, T item, IComparer<T> comparer)
[Pure]
internal int IndexOf(T item, IEqualityComparer<T> equalityComparer) => this.IndexOf(item, 0, this.Count, equalityComparer);

/// <summary>
/// Searches for the specified object and returns <c>true</c> if it is found, <c>false</c> otherwise.
/// </summary>
/// <param name="item">
/// The object to locate in the <see cref="ImmutableList{T}"/>. The value
/// can be null for reference types.
/// </param>
/// <param name="equalityComparer">
/// The equality comparer to use for testing the match of two elements.
/// </param>
/// <returns>
/// <c>true</c> if it is found, <c>false</c> otherwise.
/// </returns>
[Pure]
internal bool Contains(T item, IEqualityComparer<T> equalityComparer) => Contains(this, item, equalityComparer);

/// <summary>
/// Searches for the specified object and returns the zero-based index of the
/// first occurrence within the range of elements in the <see cref="ImmutableList{T}"/>
Expand Down Expand Up @@ -1542,6 +1558,22 @@ private static Node CreateRange(IEnumerable<T> keys)
/// <param name="key">The leaf node's key.</param>
/// <returns>The leaf node.</returns>
private static Node CreateLeaf(T key) => new Node(key, left: EmptyNode, right: EmptyNode);

/// <summary>
/// Traverses the node tree looking for a node with the provided value. The provided node will be checked
/// then we will recursively check it's left and right nodes.
/// </summary>
/// <param name="node">
/// The node to check.
/// </param>
/// <param name="value">
/// The value to check for.
/// </param>
/// <param name="equalityComparer">
/// The equality comparer to use for testing the node and value.
/// </param>
/// <returns></returns>
private static bool Contains(Node node, T value, IEqualityComparer<T> equalityComparer) => !node.IsEmpty && (equalityComparer.Equals(value, node._key) || Contains(node._left, value, equalityComparer) || Contains(node._right, value, equalityComparer));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ public ImmutableList<TOutput> ConvertAll<TOutput>(Func<T, TOutput> converter)
/// <summary>
/// See the <see cref="IImmutableList{T}"/> interface.
/// </summary>
public bool Contains(T value) => this.IndexOf(value) >= 0;
public bool Contains(T value) => _root.Contains(value, EqualityComparer<T>.Default);

/// <summary>
/// See the <see cref="IImmutableList{T}"/> interface.
Expand Down

0 comments on commit fa9a944

Please sign in to comment.