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

Precompile files #2659

Merged
merged 7 commits into from
Dec 27, 2021
Merged

Precompile files #2659

merged 7 commits into from
Dec 27, 2021

Conversation

alfonsogarciacaro
Copy link
Member

@alfonsogarciacaro alfonsogarciacaro commented Dec 15, 2021

@ncave I'm trying to add a precompile command so users can precompile common files and packages that are changed less often and speed up the compilations. For this, I need to generate a .dll assembly, so it can be referenced later from the main project. I'm using the FSharpChecker.Compile method from FCS and it works, only it takes a long time and I cannot reuse the typed AST already generated (there's an overload to use the untyped AST but for this I don't have access to it currently and not sure if it will improve times a lot as parsing is much faster than type checking).

Do you know if it would be possible/easy to integrate the exports branch of your fork into service_slim to generate empty assemblies (only with metadata)? And whether these empty assemblies can be generated faster than assemblies with code? If it doesn't look feasible/easy to do we can forget about it though because precompiling shouldn't be done often anyways :)

I'm planning to use this feature also to precompile libs for REPL so it would be useful there too.

@ncave
Copy link
Collaborator

ncave commented Dec 22, 2021

@alfonsogarciacaro Sorry for the late answer, didn't see your question. Yes, you can easily merge export and service_slim, but I don't think that will help, since export only exports already compiled and loaded/referenced assemblies, just without their code portion. So there is no shortcut there. The only benefit from that is a bit smaller binary size, that's all. And for F# assemblies that benefit is even smaller, cause they have a lot of extra metadata.

Here is what export does, basically just re-exporting the imported assemblies.

I may be entirely wrong, cause I have not measured it, but I'm not sure the code generation portion is the slow part, it's probably the type-checking that is slower.

@alfonsogarciacaro
Copy link
Member Author

Thanks for the reply @ncave! You're right, I was having a look at this and realized my first assumption was wrong. The problem is actually I'm compiling twice: one for the normal F# - Fable compilation cycle and another to generate the .dll. Ideally we should have a way to tell ParseAndCheckProject to use the results to generate a .dll. I tried passing "--out:path/to/file.dll" in the options but it didn't work. Do you know if there's a way to generate the .dll assembly from the ParseAndCheckProject results?

@ncave
Copy link
Collaborator

ncave commented Dec 22, 2021

@alfonsogarciacaro

Do you know if there's a way to generate the .dll assembly from the ParseAndCheckProject results?

I'm sure there is a way, after all that's what the F# compiler does next, generating IL code from the type-checked AST.
But I don't know whether is available as an nice API, or do we need to expose it. Perhaps you can ask in the F# compiler repo, maybe one of the maintainers there can point us in the right direction and save us some digging through the compiler code.

@alfonsogarciacaro
Copy link
Member Author

@ncave I think I got it :) Seems to work, in my tests calling compileOfTypedAst seems to take half the time of FSharpChecker.Compile. Unfortunately it seems it cannot be parallelized, at one point I tried to generate the .dll in parallel to Fable compilation but the time was added at the end. It looks like there's a lock when accessing the TypedImplFiles.

@ncave
Copy link
Collaborator

ncave commented Dec 23, 2021

@alfonsogarciacaro This is great, looks very minimal, I like it.

@ncave
Copy link
Collaborator

ncave commented Dec 23, 2021

@alfonsogarciacaro

Unfortunately it seems it cannot be parallelized

Would it help if you detach the type-checking from the IL code generation (expose the compileOfTypedAst as separate API) and then stagger (or pub-sub) the type-checking and IL compiling in the client?

Optional but nice:

  • I know you can do it within the existing API too, first call just to typecheck, then to compile IL (from another thread),
    but it's just cleaner if they're separated (but not absolutely necessary).
  • You don't have to round-trip the type-check result, you can pull it from the cache (or call the other typecheck API to generate it if missing, same as today).
  • Also, with a separate API for IL compilation you can return a tuple, so you can theoretically return the PDB too
    (if you need it).

@alfonsogarciacaro
Copy link
Member Author

Thanks for the advice @ncave, I applied it and it looks better now :)

After checking the project, I start the dll compilation as a child but it still seems it doesn't do much work until the other operations are finished. I think now it's just because the Fable compilation and the serialization of inline expressions just take up all the threads in the thread pool. If you can think of a better idea to start the work, please let me know :)

@ncave
Copy link
Collaborator

ncave commented Dec 24, 2021

@alfonsogarciacaro

Just to clarify what I mean by "stagger (or pub-sub) the type-checking and IL compiling in the client":

  • Let's assume the type-checking cannot be parallelized. In this scenario we can also remove the parallelization of the parsing to keep the type-checking API single-threaded.
  • This leaves some unused CPU while files are being type-checked, which we can use to generate IL code.
  • So I was proposing that one (producer) thread is type-checking files one by one, and after each file is type-checked, creates work for other (consumer) thread(s) to generate IL code.
  • The simplest way of doing it is probably with a blocking collection, but I'm sure it can be done with MailboxProcessor too.
  • Does that make sense? All this should happen before Fable starts transforming the AST, and, ideally, complete in the same time as before (if IL code generation is not much slower than type-checking).

@alfonsogarciacaro
Copy link
Member Author

That's a great idea @ncave! Indeed, type checking cannot be parallelized, it uses a mapFold that takes the accumulated tcState before checking each file. It's always painful for me to see always that only 25% of my 4 CPU cores are being used during type checking :)

However, a changes as suggests will likely require many more changes in the FCS pipeline as it's basically implemented in a step-by-step sequential fashion. There are several points that expect the full list of TypedImplFiles, like optimization (I tried skipping optimization but got runtime errors later in the pipeline) or IL code gen.

Not sure if this can be changed to a pub-sub pattern, but if it's possible it should be beneficial for the F# compilation itself too.

In any case, you made me think we could try to apply the pub-sub pattern to F#-Fable compilation. Ideally this would kick-off Fable compilation before type checking finishes so Fable can use the free CPU.

BTW, I just remembered about the incoming support for reference assemblies in F#. Maybe we can try rebasing with this branch and see if it's faster to generate reference assemblies (these should be enough for fable precompilation).

@ncave
Copy link
Collaborator

ncave commented Dec 26, 2021

@alfonsogarciacaro +1 for Fable always pushing the boundaries of the (b)leading edge of the F# compiler.

  • Fable: Using "reference" metadata assemblies years before it became en vogue to do so :)

But joking aside, I really hope FSharp.Core reference assembly will be smaller than what we use today.

@alfonsogarciacaro alfonsogarciacaro changed the title [WIP] Precompile files Precompile files Dec 27, 2021
@alfonsogarciacaro
Copy link
Member Author

I'm merging this because I cannot find more opportunities to improve performance at the moment. I will try the sub-pub pattern with F#-Fable compilation in a different PR.

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

Successfully merging this pull request may close these issues.

2 participants