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

Improve AsyncMemoize tests #16580

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Improve AsyncMemoize tests #16580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions src/Compiler/Facilities/AsyncMemoize.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ type internal Job<'TValue> =
| Failed(_, ex) -> $"Failed {ex}"

type internal JobEvent =
| Requested
| Started
| Restarted
| Finished
| Canceled
| Evicted
Expand Down Expand Up @@ -245,7 +247,7 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T

let cached, otherVersions = cache.GetAll(key.Key, key.Version)

return
let result =
match msg, cached with
| GetOrCompute _, Some(Completed(result, diags)) ->
Interlocked.Increment &hits |> ignore
Expand Down Expand Up @@ -298,6 +300,9 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
cts.Cancel())

New cts.Token

log (Requested, key)
return result
})

let internalError key message =
Expand Down Expand Up @@ -346,7 +351,7 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T

try
// TODO: Should unify starting and restarting
log (Started, key)
log (Restarted, key)
Interlocked.Increment &restarted |> ignore
System.Diagnostics.Trace.TraceInformation $"{name} Restarted {key.Label}"
let currentLogger = DiagnosticsThreadStatics.DiagnosticsLogger
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/Facilities/AsyncMemoize.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ module internal Utils =
val (|TaskCancelled|_|): ex: exn -> TaskCanceledException option

type internal JobEvent =
| Requested
| Started
| Restarted
| Finished
| Canceled
| Evicted
Expand Down
50 changes: 41 additions & 9 deletions tests/FSharp.Compiler.ComponentTests/CompilerService/AsyncMemoize.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ let ``Basics``() =
let groups = eventLog |> Seq.groupBy snd |> Seq.toList
Assert.Equal(3, groups.Length)
for key, events in groups do
Assert.Equal<Set<(JobEvent * int)>>(Set [ Started, key; Finished, key ], Set events)
Assert.Equal<Set<(JobEvent * int)>>(Set [ Requested, key; Started, key; Finished, key ], Set events)

[<Fact>]
let ``We can cancel a job`` () =
Expand Down Expand Up @@ -96,9 +96,14 @@ let ``We can cancel a job`` () =
waitFor jobStarted
jobStarted.Reset() |> ignore

let jobRequested = new ManualResetEvent(false)
memoize.OnEvent(fun (e, _) -> if e = Requested then jobRequested.Set() |> ignore)

let _task2 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation ignore), ct = cts2.Token)
let _task3 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation ignore), ct = cts3.Token)

waitFor jobRequested

cts1.Cancel()
cts2.Cancel()

Expand All @@ -108,7 +113,14 @@ let ``We can cancel a job`` () =

waitFor jobCanceled

Assert.Equal<(JobEvent * int) array>([| Started, key; Started, key; Canceled, key |], eventLog |> Seq.toArray )
Assert.Equal<(JobEvent * int) array>([|
Requested, key
Started, key
Requested, key
Requested, key
Restarted, key
Canceled, key
|], eventLog |> Seq.toArray )
}

[<Fact>]
Expand Down Expand Up @@ -139,22 +151,31 @@ let ``Job is restarted if first requestor cancels`` () =
waitFor jobStarted
jobStarted.Reset() |> ignore

let jobRequested = new ManualResetEvent(false)
memoize.OnEvent(fun (e, _) -> if e = Requested then jobRequested.Set() |> ignore)

let _task2 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation key), ct = cts2.Token)
let _task3 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation key), ct = cts3.Token)

waitFor jobRequested

cts1.Cancel()

waitFor jobStarted

cts3.Cancel()

jobCanComplete.Set() |> ignore

let! result = _task2
Assert.Equal(2, result)

let orderedLog = eventLog |> Seq.rev |> Seq.toList
let expected = [ Started, key; Started, key; Finished, key ]
let expected = [
Requested, key
Started, key
Requested, key
Requested, key
Restarted, key
Finished, key ]

Assert.Equal<_ list>(expected, orderedLog)
}
Expand Down Expand Up @@ -184,15 +205,20 @@ let ``Job is restarted if first requestor cancels but keeps running if second re

let _task1 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation key), ct = cts1.Token)

jobStarted.WaitOne() |> ignore
waitFor jobStarted
jobStarted.Reset() |> ignore

let jobRequested = new ManualResetEvent(false)
memoize.OnEvent(fun (e, _) -> if e = Requested then jobRequested.Set() |> ignore)

let _task2 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation key), ct = cts2.Token)
let _task3 = NodeCode.StartAsTask_ForTesting( memoize.Get'(key, computation key), ct = cts3.Token)

waitFor jobRequested

cts1.Cancel()

jobStarted.WaitOne() |> ignore
waitFor jobStarted

cts2.Cancel()

Expand All @@ -202,7 +228,13 @@ let ``Job is restarted if first requestor cancels but keeps running if second re
Assert.Equal(2, result)

let orderedLog = eventLog |> Seq.rev |> Seq.toList
let expected = [ Started, key; Started, key; Finished, key ]
let expected = [
Requested, key
Started, key
Requested, key
Requested, key
Restarted, key
Finished, key ]

Assert.Equal<_ list>(expected, orderedLog)
}
Expand All @@ -228,7 +260,7 @@ let ``Stress test`` () =
let keyCount = rng.Next(5, 200)
let keys = [| 1 .. keyCount |]

let testTimeoutMs = threads * iterations * maxDuration
let testTimeoutMs = threads * iterations * maxDuration * 2

let intenseComputation durationMs result =
async {
Expand Down
6 changes: 3 additions & 3 deletions tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ let ``File is not checked twice`` () =
|> Seq.map (fun (k, g) -> k, g |> Seq.map fst |> Seq.toList)
|> Map

Assert.Equal<JobEvent list>([Weakened; Started; Finished], intermediateTypeChecks["FileFirst.fs"])
Assert.Equal<JobEvent list>([Weakened; Started; Finished], intermediateTypeChecks["FileThird.fs"])
Assert.Equal<JobEvent list>([Weakened; Requested; Started; Finished], intermediateTypeChecks["FileFirst.fs"])
Assert.Equal<JobEvent list>([Weakened; Requested; Started; Finished], intermediateTypeChecks["FileThird.fs"])

[<Fact>]
let ``If a file is checked as a dependency it's not re-checked later`` () =
Expand All @@ -261,7 +261,7 @@ let ``If a file is checked as a dependency it's not re-checked later`` () =
|> Seq.map (fun (k, g) -> k, g |> Seq.map fst |> Seq.toList)
|> Map

Assert.Equal<JobEvent list>([Weakened; Started; Finished], intermediateTypeChecks["FileThird.fs"])
Assert.Equal<JobEvent list>([Weakened; Requested; Started; Finished; Requested], intermediateTypeChecks["FileThird.fs"])


// [<Fact>] TODO: differentiate complete and minimal checking requests
Expand Down
Loading