diff --git a/src/FSharp.Collections.Immutable/SortedMap/SortedMap.fs b/src/FSharp.Collections.Immutable/SortedMap/SortedMap.fs index 3c9a995a..2bf886b9 100644 --- a/src/FSharp.Collections.Immutable/SortedMap/SortedMap.fs +++ b/src/FSharp.Collections.Immutable/SortedMap/SortedMap.fs @@ -176,26 +176,37 @@ let containsValue (value : 'Value) (table : SortedMap<'Key, 'Value>) = checkNotNull (nameof table) table table.ContainsValue value -// exists and forall loop over the struct enumerator of the sorted map, which allocates nothing. The algorithm that the map -// modules share, over the key/value pairs of any dictionary, allocated 56 bytes per call, the boxed enumerator; LINQ Any -// and All allocated 136 bytes, and Seq.exists and Seq.forall 80 bytes (measured in Release; a measured faster path -// replaces a higher priority, D5). Dispose returns the pooled stack of the enumerator. +// exists and forall loop over the struct enumerator of the sorted map for value types, which allocates nothing. The +// algorithm that the map modules share, over the key/value pairs of any dictionary, allocated 56 bytes per call, the boxed +// enumerator; LINQ Any and All allocated 136 bytes, and Seq.exists and Seq.forall 80 bytes (measured in Release; a measured +// faster path replaces a higher priority, D5). A map with a reference type as the key or the value is walked through the +// interface instead, at the price of that boxed enumerator, 56 bytes per call: the code that all reference types share needs a +// lookup of the exact type at run time for each call of a member of the generic struct enumerator, and for 100 to 100 000 +// bindings exists and forall took 0.55 to 0.84 times as long for strings, and 0.60 to 0.69 for a string next to an integer, +// where for integers the interface took 1.3 to 1.6 times as long. Dispose returns the pooled stack of the enumerator. [] let exists (predicate : 'Key -> 'Value -> bool) (table : SortedMap<'Key, 'Value>) = checkNotNull (nameof table) table - let mutable enumerator = table.GetEnumerator () + let mutable found = false - try - let mutable found = false + if typeof<'Key>.IsValueType && typeof<'Value>.IsValueType then + let mutable enumerator = table.GetEnumerator () + + try + while not found && enumerator.MoveNext () do + let pair = enumerator.Current + found <- predicate pair.Key pair.Value + finally + enumerator.Dispose () + else + use enumerator = (table :> seq>).GetEnumerator() while not found && enumerator.MoveNext () do let pair = enumerator.Current found <- predicate pair.Key pair.Value - found - finally - enumerator.Dispose () + found [] let findKey predicate (table : SortedMap<'Key, 'Value>) = @@ -209,18 +220,25 @@ let findKey predicate (table : SortedMap<'Key, 'Value>) = [] let forall (predicate : 'Key -> 'Value -> bool) (table : SortedMap<'Key, 'Value>) = checkNotNull (nameof table) table - let mutable enumerator = table.GetEnumerator () + let mutable satisfied = true - try - let mutable satisfied = true + if typeof<'Key>.IsValueType && typeof<'Value>.IsValueType then + let mutable enumerator = table.GetEnumerator () + + try + while satisfied && enumerator.MoveNext () do + let pair = enumerator.Current + satisfied <- predicate pair.Key pair.Value + finally + enumerator.Dispose () + else + use enumerator = (table :> seq>).GetEnumerator() while satisfied && enumerator.MoveNext () do let pair = enumerator.Current satisfied <- predicate pair.Key pair.Value - satisfied - finally - enumerator.Dispose () + satisfied [] let pick (chooser : 'Key -> 'Value -> 'U voption) (table : SortedMap<'Key, 'Value>) = diff --git a/tests/FSharp.Collections.Immutable.Tests/SortedMap/SortedMapTests.fs b/tests/FSharp.Collections.Immutable.Tests/SortedMap/SortedMapTests.fs index 864a9fc8..4df88925 100644 --- a/tests/FSharp.Collections.Immutable.Tests/SortedMap/SortedMapTests.fs +++ b/tests/FSharp.Collections.Immutable.Tests/SortedMap/SortedMapTests.fs @@ -1260,3 +1260,116 @@ type SortedMapTests () = Seq.toArray (SortedMap.empty |> SortedMap.addRange pairs), "SortedMap.addRange of the mapped pairs must accept a repeated key with an equal element, as ToImmutableSortedDictionary does" ) + + // exists and forall walk a map with a reference type through the interface and a map of value types through the struct + // enumerator, and the contracts run on integers, so strings, and a string next to an integer, have tests of their own + + [] + member _.``exists and forall of string keys and values give the results of Map`` (count : int, position : string) = + let bindings = Array.init count (fun index -> struct ("x" + index.ToString (), "v" + index.ToString ())) + + let target = + match position with + | "first" -> "x0" + | "middle" -> "x" + (count / 2).ToString() + | "last" -> "x" + (count - 1).ToString() + | _ -> "absent" + + let table = SortedMap.ofArray bindings + let expected = toMap bindings + + Assert.AreEqual ( + Map.exists (fun key _ -> key = target) expected, + SortedMap.exists (fun key _ -> key = target) table, + "SortedMap.exists must find the key as Map.exists does" + ) + + Assert.AreEqual ( + Map.forall (fun key _ -> key <> target) expected, + SortedMap.forall (fun key _ -> key <> target) table, + "SortedMap.forall must reject the key as Map.forall does" + ) + + Assert.AreEqual ( + Map.exists (fun _ (value : string) -> value.Length > 1) expected, + SortedMap.exists (fun _ (value : string) -> value.Length > 1) table, + "SortedMap.exists must answer for a predicate on the values as Map.exists does" + ) + + Assert.AreEqual ( + Map.forall (fun _ (value : string) -> value.Length > 1) expected, + SortedMap.forall (fun _ (value : string) -> value.Length > 1) table, + "SortedMap.forall must answer for a predicate on the values as Map.forall does" + ) + + [] + member _.``exists and forall of a string key with an integer value give the results of Map`` (count : int) = + let bindings = Array.init count (fun index -> struct ("x" + index.ToString (), index)) + let table = SortedMap.ofArray bindings + let expected = toMap bindings + + Assert.AreEqual ( + Map.exists (fun _ value -> value = count - 1) expected, + SortedMap.exists (fun _ value -> value = count - 1) table, + "SortedMap.exists must find the value of the last binding as Map.exists does" + ) + + Assert.AreEqual ( + Map.forall (fun _ value -> value < count) expected, + SortedMap.forall (fun _ value -> value < count) table, + "SortedMap.forall must accept the values below the count as Map.forall does" + ) + + [] + member _.``exists and forall of an integer key with a string value give the results of Map`` (count : int) = + let bindings = Array.init count (fun index -> struct (index, "v" + index.ToString ())) + let table = SortedMap.ofArray bindings + let expected = toMap bindings + + Assert.AreEqual ( + Map.exists (fun key _ -> key = count - 1) expected, + SortedMap.exists (fun key _ -> key = count - 1) table, + "SortedMap.exists must find the last key as Map.exists does" + ) + + Assert.AreEqual ( + Map.forall (fun _ (value : string) -> value.Length > 1) expected, + SortedMap.forall (fun _ (value : string) -> value.Length > 1) table, + "SortedMap.forall must answer for a predicate on the values as Map.forall does" + ) + + [] + member _.``exists and forall of strings stop at the first match and the first failure`` () = + let table = SortedMap.ofArray [| struct ("a", "1"); struct ("b", "2"); struct ("c", "3") |] + let calls = ref 0 + + let found = + SortedMap.exists + (fun key _ -> + calls.Value <- calls.Value + 1 + key = "a" + ) + table + + Assert.IsTrue (found, "SortedMap.exists must find the first key in the order") + Assert.AreEqual (1, calls.Value, "SortedMap.exists must not call the predicate after the first match") + calls.Value <- 0 + + let holds = + SortedMap.forall + (fun key _ -> + calls.Value <- calls.Value + 1 + key <> "a" + ) + table + + Assert.IsFalse (holds, "SortedMap.forall must fail on the first key in the order") + Assert.AreEqual (1, calls.Value, "SortedMap.forall must not call the predicate after the first failure")