From fa9a944f9be4fd96be989efa6a3817786052c005 Mon Sep 17 00:00:00 2001 From: Nathan Mascitelli Date: Sat, 19 Oct 2019 16:44:24 -0400 Subject: [PATCH] Improve the performance of ImmutableList.Contains (dotnet/corefx#40540) * 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 https://github.com/dotnet/corefx/commit/a866d9643f23ddd2f47762e02ccdd2e387a8e5c8 --- .../Immutable/ImmutableList_1.Node.cs | 32 +++++++++++++++++++ .../Collections/Immutable/ImmutableList_1.cs | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs index f62bbf9cf593d..cfde5012e0f65 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.Node.cs @@ -704,6 +704,22 @@ internal int BinarySearch(int index, int count, T item, IComparer comparer) [Pure] internal int IndexOf(T item, IEqualityComparer equalityComparer) => this.IndexOf(item, 0, this.Count, equalityComparer); + /// + /// Searches for the specified object and returns true if it is found, false otherwise. + /// + /// + /// The object to locate in the . The value + /// can be null for reference types. + /// + /// + /// The equality comparer to use for testing the match of two elements. + /// + /// + /// true if it is found, false otherwise. + /// + [Pure] + internal bool Contains(T item, IEqualityComparer equalityComparer) => Contains(this, item, equalityComparer); + /// /// Searches for the specified object and returns the zero-based index of the /// first occurrence within the range of elements in the @@ -1542,6 +1558,22 @@ private static Node CreateRange(IEnumerable keys) /// The leaf node's key. /// The leaf node. private static Node CreateLeaf(T key) => new Node(key, left: EmptyNode, right: EmptyNode); + + /// + /// 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. + /// + /// + /// The node to check. + /// + /// + /// The value to check for. + /// + /// + /// The equality comparer to use for testing the node and value. + /// + /// + private static bool Contains(Node node, T value, IEqualityComparer equalityComparer) => !node.IsEmpty && (equalityComparer.Equals(value, node._key) || Contains(node._left, value, equalityComparer) || Contains(node._right, value, equalityComparer)); } } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.cs index 55059b7ee08ec..7ed9ce2cbafdd 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableList_1.cs @@ -812,7 +812,7 @@ public ImmutableList ConvertAll(Func converter) /// /// See the interface. /// - public bool Contains(T value) => this.IndexOf(value) >= 0; + public bool Contains(T value) => _root.Contains(value, EqualityComparer.Default); /// /// See the interface.