Skip to content

Commit ef8571c

Browse files
authored
Merge pull request #292 from fsprojects/repo-assist/test-perf-improve-2026-03-0c358a89d8e70d4c
[Repo Assist] perf/test: replace ConcurrentHashSet with plain HashSet in except; add zip/indexed tests
2 parents f8789d1 + 4a3fedf commit ef8571c

File tree

3 files changed

+136
-58
lines changed

3 files changed

+136
-58
lines changed

src/FSharp.Control.TaskSeq.Test/TaskSeq.Indexed.Tests.fs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,68 @@ module Immutable =
3434
|> TaskSeq.toArrayAsync
3535
|> Task.map (Array.forall (fun (x, y) -> x + 1 = y))
3636
|> Task.map (should be True)
37+
38+
[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
39+
let ``TaskSeq-indexed returns all 10 pairs with correct zero-based indices`` variant = task {
40+
let! pairs =
41+
Gen.getSeqImmutable variant
42+
|> TaskSeq.indexed
43+
|> TaskSeq.toArrayAsync
44+
45+
pairs |> should be (haveLength 10)
46+
47+
pairs
48+
|> Array.iteri (fun pos (idx, _) -> idx |> should equal pos)
49+
}
50+
51+
[<Theory; ClassData(typeof<TestImmTaskSeq>)>]
52+
let ``TaskSeq-indexed returns values 1 to 10 unchanged`` variant = task {
53+
let! pairs =
54+
Gen.getSeqImmutable variant
55+
|> TaskSeq.indexed
56+
|> TaskSeq.toArrayAsync
57+
58+
pairs |> Array.map snd |> should equal [| 1..10 |]
59+
}
60+
61+
module SideEffects =
62+
[<Theory; ClassData(typeof<TestSideEffectTaskSeq>)>]
63+
let ``TaskSeq-indexed on side-effect sequence returns correct pairs`` variant = task {
64+
let ts = Gen.getSeqWithSideEffect variant
65+
let! pairs = ts |> TaskSeq.indexed |> TaskSeq.toArrayAsync
66+
pairs |> should be (haveLength 10)
67+
68+
pairs
69+
|> Array.iteri (fun pos (idx, _) -> idx |> should equal pos)
70+
}
71+
72+
[<Theory; ClassData(typeof<TestSideEffectTaskSeq>)>]
73+
let ``TaskSeq-indexed on side-effect sequence is re-evaluated on second iteration`` variant = task {
74+
let ts = Gen.getSeqWithSideEffect variant
75+
76+
let! firstPairs = ts |> TaskSeq.indexed |> TaskSeq.toArrayAsync
77+
let! secondPairs = ts |> TaskSeq.indexed |> TaskSeq.toArrayAsync
78+
79+
// indices always start at 0
80+
firstPairs |> Array.map fst |> should equal [| 0..9 |]
81+
secondPairs |> Array.map fst |> should equal [| 0..9 |]
82+
83+
// values advance due to side effects
84+
firstPairs |> Array.map snd |> should equal [| 1..10 |]
85+
secondPairs |> Array.map snd |> should equal [| 11..20 |]
86+
}
87+
88+
[<Fact>]
89+
let ``TaskSeq-indexed prove index starts at zero even after side effects`` () = task {
90+
let mutable counter = 0
91+
92+
let ts = taskSeq {
93+
for _ in 1..5 do
94+
counter <- counter + 1
95+
yield counter
96+
}
97+
98+
let! pairs = ts |> TaskSeq.indexed |> TaskSeq.toArrayAsync
99+
pairs |> Array.map fst |> should equal [| 0..4 |]
100+
pairs |> Array.map snd |> should equal [| 1..5 |]
101+
}

src/FSharp.Control.TaskSeq.Test/TaskSeq.Zip.Tests.fs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,61 @@ module Performance =
118118
combined |> Array.last |> should equal (length, length)
119119
}
120120

121+
module UnequalLength =
122+
[<Fact>]
123+
let ``TaskSeq-zip stops at shorter first sequence`` () = task {
124+
// documented: "when one sequence is exhausted any remaining elements in the other sequence are ignored"
125+
let short = taskSeq { yield! [ 1..5 ] }
126+
let long = taskSeq { yield! [ 1..10 ] }
127+
let! combined = TaskSeq.zip short long |> TaskSeq.toArrayAsync
128+
combined |> should be (haveLength 5)
129+
130+
combined
131+
|> should equal (Array.init 5 (fun i -> i + 1, i + 1))
132+
}
133+
134+
[<Fact>]
135+
let ``TaskSeq-zip stops at shorter second sequence`` () = task {
136+
// documented: "when one sequence is exhausted any remaining elements in the other sequence are ignored"
137+
let long = taskSeq { yield! [ 1..10 ] }
138+
let short = taskSeq { yield! [ 1..3 ] }
139+
let! combined = TaskSeq.zip long short |> TaskSeq.toArrayAsync
140+
combined |> should be (haveLength 3)
141+
142+
combined
143+
|> should equal (Array.init 3 (fun i -> i + 1, i + 1))
144+
}
145+
146+
[<Fact>]
147+
let ``TaskSeq-zip with first sequence empty returns empty`` () =
148+
// documented: remaining elements in the longer sequence are ignored
149+
let empty = taskSeq { yield! ([]: int list) }
150+
let nonEmpty = taskSeq { yield! [ 1..10 ] }
151+
TaskSeq.zip empty nonEmpty |> verifyEmpty
152+
153+
[<Fact>]
154+
let ``TaskSeq-zip with second sequence empty returns empty`` () =
155+
// documented: remaining elements in the longer sequence are ignored
156+
let nonEmpty = taskSeq { yield! [ 1..10 ] }
157+
let empty = taskSeq { yield! ([]: int list) }
158+
TaskSeq.zip nonEmpty empty |> verifyEmpty
159+
160+
[<Fact>]
161+
let ``TaskSeq-zip with singleton first and longer second returns singleton`` () = task {
162+
let one = taskSeq { yield 42 }
163+
let many = taskSeq { yield! [ 1..10 ] }
164+
let! combined = TaskSeq.zip one many |> TaskSeq.toArrayAsync
165+
combined |> should equal [| (42, 1) |]
166+
}
167+
168+
[<Fact>]
169+
let ``TaskSeq-zip with longer first and singleton second returns singleton`` () = task {
170+
let many = taskSeq { yield! [ 1..10 ] }
171+
let one = taskSeq { yield 99 }
172+
let! combined = TaskSeq.zip many one |> TaskSeq.toArrayAsync
173+
combined |> should equal [| (1, 99) |]
174+
}
175+
121176
module Other =
122177
[<Fact>]
123178
let ``TaskSeq-zip zips different types`` () = task {

src/FSharp.Control.TaskSeq/TaskSeqInternal.fs

Lines changed: 16 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -975,58 +975,7 @@ module internal TaskSeqInternal =
975975
raiseOutOfBounds (nameof index)
976976
}
977977

978-
// Consider turning using an F# version of this instead?
979-
// https://github.com/i3arnon/ConcurrentHashSet
980-
type ConcurrentHashSet<'T when 'T: equality>(ct) =
981-
let _rwLock = new ReaderWriterLockSlim()
982-
let hashSet = HashSet<'T>(Array.empty, HashIdentity.Structural)
983-
984-
member _.Add item =
985-
_rwLock.EnterWriteLock()
986-
987-
try
988-
hashSet.Add item
989-
finally
990-
_rwLock.ExitWriteLock()
991-
992-
member _.AddMany items =
993-
_rwLock.EnterWriteLock()
994-
995-
try
996-
for item in items do
997-
hashSet.Add item |> ignore
998-
999-
finally
1000-
_rwLock.ExitWriteLock()
1001-
1002-
member _.AddManyAsync(source: TaskSeq<'T>) = task {
1003-
use e = source.GetAsyncEnumerator(ct)
1004-
let mutable go = true
1005-
let! step = e.MoveNextAsync()
1006-
go <- step
1007-
1008-
while go do
1009-
// NOTE: r/w lock cannot cross thread boundaries. Should we use SemaphoreSlim instead?
1010-
// or alternatively, something like this: https://github.com/StephenCleary/AsyncEx/blob/8a73d0467d40ca41f9f9cf827c7a35702243abb8/src/Nito.AsyncEx.Coordination/AsyncReaderWriterLock.cs#L16
1011-
// not sure how they compare.
1012-
1013-
_rwLock.EnterWriteLock()
1014-
1015-
try
1016-
hashSet.Add e.Current |> ignore
1017-
finally
1018-
_rwLock.ExitWriteLock()
1019-
1020-
let! step = e.MoveNextAsync()
1021-
go <- step
1022-
}
1023-
1024-
interface IDisposable with
1025-
override _.Dispose() =
1026-
if not (isNull _rwLock) then
1027-
_rwLock.Dispose()
1028-
1029-
let except itemsToExclude (source: TaskSeq<_>) =
978+
let except (itemsToExclude: TaskSeq<_>) (source: TaskSeq<_>) =
1030979
checkNonNull (nameof source) source
1031980
checkNonNull (nameof itemsToExclude) itemsToExclude
1032981

@@ -1037,9 +986,18 @@ module internal TaskSeqInternal =
1037986
go <- step
1038987

1039988
if step then
1040-
// only create hashset by the time we actually start iterating
1041-
use hashSet = new ConcurrentHashSet<_>(CancellationToken.None)
1042-
do! hashSet.AddManyAsync itemsToExclude
989+
// only create hashset by the time we actually start iterating;
990+
// taskSeq enumerates sequentially, so a plain HashSet suffices — no locking needed.
991+
let hashSet = HashSet<_>(HashIdentity.Structural)
992+
993+
use excl = itemsToExclude.GetAsyncEnumerator CancellationToken.None
994+
let! exclStep = excl.MoveNextAsync()
995+
let mutable exclGo = exclStep
996+
997+
while exclGo do
998+
hashSet.Add excl.Current |> ignore
999+
let! exclStep = excl.MoveNextAsync()
1000+
exclGo <- exclStep
10431001

10441002
while go do
10451003
let current = e.Current
@@ -1065,9 +1023,9 @@ module internal TaskSeqInternal =
10651023
go <- step
10661024

10671025
if step then
1068-
// only create hashset by the time we actually start iterating
1069-
use hashSet = new ConcurrentHashSet<_>(CancellationToken.None)
1070-
do hashSet.AddMany itemsToExclude
1026+
// only create hashset by the time we actually start iterating;
1027+
// initialize directly from the seq — taskSeq is sequential so no locking needed.
1028+
let hashSet = HashSet<_>(itemsToExclude, HashIdentity.Structural)
10711029

10721030
while go do
10731031
let current = e.Current

0 commit comments

Comments
 (0)