Skip to content

Conversation

@kerams
Copy link
Contributor

@kerams kerams commented Aug 25, 2023

I suggest you hide whitespace when reviewing.

@kerams kerams requested a review from a team as a code owner August 25, 2023 16:43
@edgarfgp
Copy link
Contributor

I was actually about to do this. Glad I checked first :)

Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@vzarytovskii
Copy link
Member

I feel like we can also inline a bunch of stuff in the builder, so it won't be wasting time on calls.

Comment on lines 696 to +698
trackErrors {
for tpr, _ in vs do
return! SolveTypStaticReqTypar csenv trace req tpr
do! SolveTypStaticReqTypar csenv trace req tpr
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this doesn't cause an early return on the first item in the loop, so do! should be clearer.

@kerams
Copy link
Contributor Author

kerams commented Aug 25, 2023

Benchmark

open BenchmarkDotNet.Attributes
open BenchmarkDotNet.Configs
open System.Diagnostics

[<NoEquality; NoComparison>]
type OperationResult<'T> =
    | OkResult of warnings: exn list * result: 'T
    | ErrorResult of warnings: exn list * error: exn

let inline ErrorD err = ErrorResult([], err)

let inline WarnD err = OkResult([ err ], ())

let inline CompleteD() = OkResult([], ())

let inline ResultD x = OkResult([], x)

[<DebuggerHidden; DebuggerStepThrough>]
let bind f res =
    match res with
    | OkResult ([], res) -> (* tailcall *) f res
    | OkResult (warns, res) ->
        match f res with
        | OkResult (warns2, res2) -> OkResult(warns @ warns2, res2)
        | ErrorResult (warns2, err) -> ErrorResult(warns @ warns2, err)
    | ErrorResult (warns, err) -> ErrorResult(warns, err)

[<DebuggerHidden; DebuggerStepThrough>]
let inline bindI f res =
    match res with
    | OkResult ([], res) -> (* tailcall *) f res
    | OkResult (warns, res) ->
        match f res with
        | OkResult (warns2, res2) -> OkResult(warns @ warns2, res2)
        | ErrorResult (warns2, err) -> ErrorResult(warns @ warns2, err)
    | ErrorResult (warns, err) -> ErrorResult(warns, err)

type TrackErrorsBuilderVanilla() =
    member x.Bind(res, k) = bind k res
    member x.Return res = ResultD res
    member x.ReturnFrom res = res
    member x.Combine(expr1, expr2) = bind expr2 expr1
    member x.Zero() = CompleteD ()
    member x.Delay fn = fun () -> fn ()
    member x.Run fn = fn ()

type TrackErrorsBuilderNoClosure() =
    member x.Bind(res, k) = bind k res
    member x.Return res = ResultD res
    member x.ReturnFrom res = res
    member x.Combine(expr1, expr2) = bind expr2 expr1
    member x.Zero() = CompleteD ()
    member x.Delay fn = fn
    member x.Run fn = fn ()

type TrackErrorsBuilderInline() =
    member inline x.Bind(res, k) = bind k res
    member inline x.Return res = ResultD res
    member inline x.ReturnFrom res = res
    member inline x.Combine(expr1, expr2) = bind expr2 expr1
    member inline x.Zero() = CompleteD ()
    member inline x.Delay fn = fn
    member inline x.Run fn = fn ()

type TrackErrorsBuilderInlineDouble() =
    member inline x.Bind(res, k) = bindI k res
    member inline x.Return res = ResultD res
    member inline x.ReturnFrom res = res
    member inline x.Combine(expr1, expr2) = bindI expr2 expr1
    member inline x.Zero() = CompleteD ()
    member inline x.Delay fn = fn
    member inline x.Run fn = fn ()


[<MemoryDiagnoser>]
type InterpolateBenchmarks () =
    let vanilla = TrackErrorsBuilderVanilla ()
    let noClosure = TrackErrorsBuilderNoClosure ()
    let inlin = TrackErrorsBuilderInline ()
    let inlin2 = TrackErrorsBuilderInlineDouble ()
    let CompleteD = CompleteD ()
    let e = System.Exception ()

    [<Benchmark>]
    member this.Vanilla () =
        vanilla {
            do! CompleteD
            do! CompleteD
            do! CompleteD
            return! WarnD e
        }

    [<Benchmark>]
    member this.NoClosure () =
        noClosure {
            do! CompleteD
            do! CompleteD
            do! CompleteD
            return! WarnD e
        }

    [<Benchmark>]
    member this.Inline () =
        inlin {
            do! CompleteD
            do! CompleteD
            do! CompleteD
            return! WarnD e
        }

    [<Benchmark>]
    member this.Inline2 () =
        inlin2 {
            do! CompleteD
            do! CompleteD
            do! CompleteD
            return! WarnD e
        }

BenchmarkDotNet.Running.BenchmarkRunner.Run (
    typeof<InterpolateBenchmarks>.Assembly,
    DefaultConfig.Instance.WithOption (ConfigOptions.JoinSummary, true))
|> ignore


BenchmarkDotNet v0.13.7, Windows 11 (10.0.22621.2134/22H2/2022Update/SunValley2)
AMD Ryzen 9 7900, 1 CPU, 24 logical and 12 physical cores
.NET SDK 8.0.100-preview.7.23376.3
  [Host]     : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2 DEBUG
  DefaultJob : .NET 8.0.0 (8.0.23.37506), X64 RyuJIT AVX2


Method Mean Error StdDev Gen0 Allocated
Vanilla 28.271 ns 0.5237 ns 0.4642 ns 0.0110 184 B
NoClosure 22.264 ns 0.3106 ns 0.2905 ns 0.0081 136 B
Inline 23.497 ns 0.2645 ns 0.2474 ns 0.0081 136 B
Inline2 8.453 ns 0.1718 ns 0.1523 ns 0.0038 64 B

The only downside with inlining bind is the fact that we get copies of the method below for every single bind operation in the CE.

image

[<InlineIfLambda>] is also an option in bind - the closure copies disappear and the entire CE body is then contained in the benchmarked method. It does not really perform any faster though.

@kerams
Copy link
Contributor Author

kerams commented Aug 27, 2023

I'm seeing allocations go from ~117MB to ~115MB in DecentlySizedStandAloneFileBenchmark. Who knows how reliable those numbers are though. I get slightly different byte counts between identical runs for whatever reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants