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.

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

F# compilation very slow with FCS v41.0.1 #2605

Open
alfonsogarciacaro opened this issue Nov 11, 2021 · 14 comments
Open

F# compilation very slow with FCS v41.0.1 #2605

alfonsogarciacaro opened this issue Nov 11, 2021 · 14 comments

Comments

@alfonsogarciacaro
Copy link
Member

I didn't notice any difference when compiling the tests, but after switching the custom FCS build with the v41.0.1 Nuget package and testing it with a big project (1275 files and I believe it heavily uses SRTP) I realized the F# compilation is very slow, it takes almost 2 times as long as with current Fable version (3.4.10, note that Fable with FCS Nuget hasn't been yet released:

Fable 3.4.10

F# compilation finished in 65379ms
Fable compilation finished in 15852ms

main branch (actually queue-watch)

F# compilation finished in 102581ms
Fable compilation finished in 17020ms

The F# compilation time only measures the ParseAndCheckProject call so the performance decrease must come from FCS, unless I'm initializing the options or the checker in a wrong way.

@ncave I guess it will be difficult to say with so little information, but do you have any idea where the problem may come from? Latest main is built with dotnet sdk 6 but targets net5.0 (I did try to target net6.0 but there was no difference). I remember you pruned off some features in service_slim but don't remember if this also impacted performance or only memory. Should I try to create a repro and report the issue in the dotnet/fsharp repo (problem is the project mentioned above is not public)?

@ncave
Copy link
Collaborator

ncave commented Nov 11, 2021

@alfonsogarciacaro Do you see any difference compiling FCS-Fable itself? If not, then it could be some difference for certain F# constructs, in recent FCS package, perhaps you can try older versions of the FCS package to see if that's true.

@alfonsogarciacaro
Copy link
Member Author

@ncave You were right, the performance decrease seems to have happened already in a previous release, specifically in v40.0.1-preview. I also tried building standalone but I keep getting this error even with FCS v40.0, which maybe related to the hack you had to make in your fork for Witness issues. It's weird because I think at one point it was working but maybe I made a mistake when testing.

At one point I also tried to create a custom build with latest dotnet/fsharp/main, but I'm getting errors about packages missing from custom Nuget feeds. I missed this brief moment when it was very easy to build FSC :)

We probably should defer the F# 6 upgrade until we can investigate this more, so going back to your fork for now :)

I've reported this in dotnet/fsharp repo: dotnet/fsharp#12380

@ncave
Copy link
Collaborator

ncave commented Nov 12, 2021

@alfonsogarciacaro Can you use v40.0 for now?

@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented Nov 12, 2021

@ncave I was trying to reenable standalone again, but now I'm getting some errors related to how I'm dealing with inline functions now :) I'm trying to collect now the inline info in advance same as we do with entities. I should fix those anyways.

Interestingly, when using your fork and the "standard" FCS API I still get worse performance (this commit):

Compiling big project:
=====================
Fable 3.4.10: F# compilation finished in 63421ms
Commit 22bfb482e1c561c43baea8499727ac61ad90254f: F# compilation finished in 78840ms

Compiling Standalone:
=====================
Fable 3.4.10: F# compilation finished in 18844ms
Commit 22bfb482e1c561c43baea8499727ac61ad90254f: F# compilation finished in 24648ms

So there's benefit in keeping the service_slim, but I need some fixes (like adding SourceIdentifier here) so now I've to make a fork of your fork :)

@ncave
Copy link
Collaborator

ncave commented Nov 12, 2021

@alfonsogarciacaro Sounds like you're saying there may be still some benefits from having a facade on top of FCS. I'll try to find some time to look at updating the fork.

@alfonsogarciacaro
Copy link
Member Author

@ncave Seems so. I had some success in rebasing service_slim with dotnet/fsharp up to 2021-06-08 (which contains this fix). I managed to build FCS by using this command and I also tried adding a lastFile argument to ParseAndCheckProject. I was hoping this would avoid the current worst case in watch compilation in Fable (touching a file up in the list triggers checking for all files below even if it's a leaf node) but I'm not getting the expected results (now touching the last file takes much longer than before). Still investigating :/

@ncave
Copy link
Collaborator

ncave commented Nov 12, 2021

@alfonsogarciacaro I thought watch is now using ParseAndCheckFileInProject, no?

@dsyme
Copy link

dsyme commented Nov 12, 2021

If we can get any profiling information for comparison that would be great.

@alfonsogarciacaro
Copy link
Member Author

@ncave Yes, but at the end ParseAndCheckFileInProject checks all files before the updated one, and because I know the bulk of changes in each watch iteration (the changed files plus the ones depending on them from Fable's dependency tree), it's easier (and I assume it saves work) if I just tell FCS until what file it needs to recompile.

Thanks a lot for checking this @dsyme! I replied in the dotnet/fsharp issue 👍

@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented Nov 12, 2021

Not entirely related, but some interesting data. With current main I get these timings:

  • Compiling standalone (FCS + Fable itself)

    • F# compilation finished in 18105ms
    • Fable compilation finished in 22801ms
  • Compiling big project (1275 files)

    • F# compilation finished in 64787ms
    • Fable compilation finished in 12285ms

So with standalone Fable compilation takes longer but with the big project is much faster (this is often the case). I wonder what's causing this difference, but I think this big project uses SRTP extensively (FCS resolves these for Fable now, so that would explain why it takes longer).

UPDATE: Another possibility is that files in FCS+Fable are very big (in they other big project they're much smaller in general) so the parallelization in Fable compilation is less effective.

@ncave
Copy link
Collaborator

ncave commented Nov 12, 2021

@alfonsogarciacaro Can you post the same numbers with Fable 3.4.10, for comparison?

@alfonsogarciacaro
Copy link
Member Author

@ncave Here you go:

  • Compiling standalone (FCS + Fable itself) with Fable 3.4.10

    • F# compilation finished in 18844ms
    • Fable compilation finished in 25367ms
  • Compiling big project (1275 files) with Fable 3.4.10

    • F# compilation finished in 61553ms
    • Fable compilation finished in 11920ms

They're very similar which makes sense because at the end main is using a version of FCS very close to 3.4.10. Also, as you can see here, FCS v40.0 from Nuget is still a bit slower that service_slim (I've updated the comment to include standalone timing).

@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented Nov 14, 2021

Some interesting things happening. I managed to update ncave's service_slim to latest dotnet/fsharp. See #2608

I did the update in steps trying to find the point when performance dropped, but I couldn't find it. It's a bit slower with latest code which seems to correspond to F# 6 doing more thing but the difference is much smaller that when using FCS API from v41.0 from Nuget. Some timings:

2021-06-21 - Standalone
F# compilation finished in 16338ms
Fable compilation finished in 19487ms

2021-07-03 - Standalone
FCS: Parsing finished in 1675ms
FCS: Checking finished in 14383ms
F# compilation finished in 16534ms
Fable compilation finished in 20158ms

2021-07-03 - Big project
FCS: Parsing finished in 1325ms
FCS: Checking finished in 60134ms
F# compilation finished in 62286ms
Fable compilation finished in 14907ms

2021-11-12 - Standalone
FCS: Parsing finished in 1749ms
FCS: Checking finished in 18818ms
F# compilation finished in 21022ms
Fable compilation finished in 19422ms

2021-11-12 - Big project
FCS: Parsing finished in 1333ms
FCS: Checking finished in 63472ms
F# compilation finished in 65616ms
Fable compilation finished in 14243ms

Note that I've parallelized the Parsing step in service_slim. This seems to help but doesn't have a big effect overall because TypeCheck is what takes most of the time.

So it looks like the performance decrease doesn't come from the compiler itself but from some bottleneck in service.fs. I'll try to investigate but at least the performance with service_slim is good enough to update Fable to F# 6 👍

@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented Nov 15, 2021

Scratch the part about parallelizing the parse phase. This can be done in the first compilation, but after that it messes up with cache invalidation so I've removed it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants