-
Notifications
You must be signed in to change notification settings - Fork 783
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.
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
Inline ValueOption module #15709
Inline ValueOption module #15709
Conversation
For the default scenarios (DefaultWithSingletonNone, DefaultWithNone), doing |
I don't know. It's not massively slower in absolute terms, and who knows what the JIT would produce if the caller function and/or lambda bodies were larger. I'd use the attribute too, if only for consistency's sake. |
I think JIT should be fine in general. It should'be be a HUGE lambda for it to not inline. We probably can remove IILAttribute for the singleton version, with comment that it was slower, I guess? |
No, because we're talking about one |
Wait, that's what I meant, remove attribute for the |
Ok, I was confused by the mention of singleton. |
I'm a bit slow today, so it's possible that I thought of one thing, and wrote a different one. |
I ran the same benchmark and added also cases with a lot larger lambda body, as well as a case which maps over ValueSome (therefore never invoking the lambda/code). The => Would prefer to have the attr removed (as is now with the latest commit) |
Continuing with #14927 and https://github.com/fsharp/fslang-design/blob/main/RFCs/FS-1115-InlineIfLambda-in-FSharp-Core.md.
Benchmark 1
Code
Decompiled
Benchmark 2
Code
Decompiled
Analysis
My conclusion is identical to #14927 - do use both inline and
InlineIfLambda
. However, there is also one important anomaly to be aware of, as highlighted between the 2 sets of benchmarks.ValueOption<int>
functions are namely surprisingly slow, to the point that they might be slower thanOption<int>
despite the latter having to allocate. This is due to a deficiency in JIT, where it produces suboptimal machine code when structs are smaller than the register size. When we switch toValueOption<int64>
,ValueOption
is clearly the winner compared toOption
. The difference between status quo and inlined functions becomes even more stark (several orders of magnitude), because the non-inlined functions take a significant performance hit withint64
.This is something to keep in mind when benchmarking fsharp/fslang-suggestions#739.