-
Notifications
You must be signed in to change notification settings - Fork 31
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
Compilation time degrades x3 to x4 times when consuming project targets net7.0 or net8.0 (rather than net6.0) #146
Comments
Hi @DunetsNM @vzarytovskii do you have any idea of what could slow down in net7/8 ? |
Yes, static abstracts introduction (both compiler feature and them being in BCL) slows it down significantly. Especially in traits heavy code. It now needs to check so many more things if they satisfy traits. |
I see, I knew about that, but I didn't know the slow down was so bad (about 3x). @DunetsNM if codecs in your codebase don't heavily use SRTPs (implicit codecs), it could be Fleece internals. If so, I can create a branch hardcoding some internal stuff, and therefore removing static resolution in the library. |
Please create issue over at compiler repo, with build instructions, I will extract profiles for us to see what exactly changed. |
Thanks @vzarytovskii @DunetsNM can you contact me by FSSF Slack so we discuss what's the best approach to follow? |
thanks for quick replies guys, I got sick today, will create a smaller repro when I get better / soon.
Correct, nearly everything is explicit |
Extra compilation time is pretty obvious even in a simple flat list of types with codecs - although only x2, perhaps it gets worse if there's a lot of nesting? Here's a dummy example: For better readability contents of the files: Types.fs
Net6Client.fsproj
Net7Cilent.fsproj (one line diff)
global.json (SDK 7.0.300 performs similar)
If you run
it takes ~3.5 sec for net6 vs ~7 sec for net7 on my machine, YMMV |
Also if you try to target So, if any SRTP-heavy code is affected, is there anything that can be done about it / apart from pinning to net6 ? I mean something like this:
I guess it depends on where the bottleneck is. Is it emitting of the inlined code (which can't be avoided in case of SRTP , but not required for IWSAM), or computationally-heavy type resolution. |
I see now, in your last code snippet you mean using IWSAMs for implicit codecs, one important limitation with that approach is retrofitting primitive types. That's not possible with IWSAMs unless you control the source code. |
@gusty yes I've played with #132 before (didn't switch because our code still requires AdHocEncoding), just double check that compilation speed difference is about the same. But it uses IWSAM only for Also, you don't really need my examples, the Fleece library itself is affected: FleeceNet6.fsproj
FleeceNet7.fsproj
Here Let me try the same with |
So for current net6.0 1:46 |
@vzarytovskii I had no luck reproducing slowdown of SRTP / inline with dummy vanilla code, it's only Fleece and/or FSharpPlus projects where slowdown in build time is obvious. Dissecting it further to find the bottleneck is not an obvious task to me. Just in case here's my dumb inline-heavy SRTP code that compiles in about the same time for net6, net7 and net8 (~7 seconds in all cases) Contents of the source file:
|
Sure, still report it, but with project examples. |
@DunetsNM thanks for all the repros. We can help @vzarytovskii to investigate and hopefully fix whatever is causing that bottleneck in the compiler but as I pointed at the beginning I am a bit surprised since I know your codebase and AFAIK it doesn't rely that much on heavy SRTP. However after seeing your repro I found that there is a small detail that causes an unnecessary level of genericity, but good news, it's fixable. The problem is this line:
Well, not that line alone, but the issue is that codecs use the This is something we're planning to change in v2 of the library, to avoid precisely situations like this. It is still not clear what would be the new namespace design (suggestions are welcome). So, it's a bit tricky right now, since the code in the end routes to the same implementation but it takes longer to compile, so you don't notice that something breaks or so. But making sure FSharpPlus is not open at the codec definition should fix the long compilation issue. If just a few functions are needed from FSharpPlus you can simply fully qualify it, for instance in your repro you can do:
... or create a small namespace in your project aliasing these functions. Another alternative could be doing exactly the opposite, I mean, keep opening FSharpPlus but open later another namespace which shadows the As said, we'll fix this in v2 of F#+ as it will be focusing more on less generic code by default. Most likely you'll have to explicitly open a "generic operators" namespace. |
The fact that the change to target framework alone changes it, leads me to believe that change in the interface hierarchy causes it |
@vzarytovskii I'll try to isolate the compiler issue so you have a smaller repro, but it doesn't seem to be easy. Still I'll give it a try. |
@DunetsNM I had a typo in my previous response, I meant the |
@dsyme just made this commit to the IWSAM RFC (fsharp/fslang-design@71cd2e0).
This suggests that @vzarytovskii's comment is probably spot on indeed. |
My comment was based on Vlad's analysis, and doesn't add new information. |
Oh, oops 😆 |
@DunetsNM Could you confirm whether my suggestion fixed the issue or not? |
@gusty somehow missed that, will try shortly. I suppose you mean these suggestions?
and
|
@gusty it helped ! The only change I had to make in my slower_net7.zip example was to remove Now both net6 and net7 take ~3.5 sec to compile. And in my prod project the problem was literally just in one
I'm not sure if we can single out It's also interesting whether compiler can help to catch such problem early and emit warning if some symbol resolution takes a very very long time. |
Ok besides the So it's definitely the naming clash between the two libraries, I can only guess what heavy lifting compiler performs while deciding to inline one operator over another. Somehow my hunch is that it's still the Fleece issue, and an operator name collision between FSharpPlus and a simpler library would not affect compilation time as much. Maybe we should simply give Fleece operators unique names, or provide plain function alternatives? |
We can summarize this issue as a combination of 4 things: 1 - F# ALWAYS treats let bound operators with higher priority than static member operators It is enough to remove one of the 4 facts and the issue is gone, but this is like aircraft accidents: not a single but a chain of events together which cause the issue. @DunetsNM I think for a "medium term" solution, (see below) changing Fleece operators is not that great, it is good that they match, so it reflects they are effectively the same (only less generic), moreover many types in FSharpPlus.Data use the same approach: operators defined at the type which are the same as the generic ones, but of course less generics. I can think of 3 actions: Short term solution: As I suggested, maybe the last option is better than fully qualifying, what I mean you can keep
then right before defining your codecs you Medium term solution: Finish and release FSharpPlus v2, without autoopening operators. This would solve this issue, but it's not that great for other uses of F#+, because you will have to do something like Long term solution: Create a suggestion, RFC and implement something (maybe an attribute?) in the F# compiler which allows to define let-bound operators with lower priority than static members, as I can see this problem will affect code outside FSharpPlus in the long run. @vzarytovskii , @dsyme what do you think? |
Besides the clash of When Fortunately there's not so many generic types with codecs where truly unconstrained type parameter is required, however even when it's not required based on usages sometimes it's simply not enforced in type signature which makes To work around that I have a custom function that is less universal than
|
I guess that would still need generic resolution for primitive types inside Here there's no way around, so far the only mechanism available in F# to retrofit an interface-like is doing this kind of complex SRTP overloads, at least until FS-1043 is finished and released. BTW: Can we close this issue, since the problem was identified and a solution (actually more than one) proposed? We should continue this in FSharpPlus v2 and also open a new F# compiler issue (and maybe a suggestion for the operators scope), linking this one. |
yes I guess we can close it with "workaround exists" |
Yes, we can conclude the main issue is the F# compiler degradation, you were lucky you were not relaying in generic code, only by accident. |
I have a project with a lot of complex Fleece codecs.
Codecs have never been too fast to compile (due to heavy usage of SRTP) but after I tried to upgrade
<TargetFramework>
value fromnet6.0
tonet7.0
ornet8.0
it got worse.The project used to take 35 to 45 seconds to compile, with net7.0 it deteriorated to circa 2:30, similar number for net8.0
It doesn't look like it's related to some specific codecs, just sheer amount of them. The more I delete/comment out the faster compilation becomes (dropping to just 15 seconds when all codecs are removed - same number for any target, net6/net7/net8 i.e. slowness is 100% codec-related).
There were many changed parts between net6 and net7/net8 versions of my projects (e.g. different versions of FSharp.Core and FSharpPlus, newer SDK) but apparently the single cause of compile time degradation is the TargetFramework value: if I change it back to
net6.0
while keeping all other changes, compilation time improves / goes back to normal.I tried to fork
Fleece
, added net7.0 and net8.0 targets to it and consumed from my project. It did not help.The main suspect is SRTP and inlining which is heavily used in codecs. Did something change in newer F# versions that made it so much slower to compile on average?
The text was updated successfully, but these errors were encountered: