facebook / facebook/buck2

Help getting precompiled header support into the prelude

Open
#877 0 comments 0 reactions 0 assignees View on GitHub
prelude windows
Dominant language
Rust
Stars
4.4k
Forks
394
PR merge metrics
No merged PRs in 30d

Description

I've talked to a few people in the past about this, but I've never gotten enough concrete detail or guidance to be able to get this to work. Possible I'm not smart enough :) The TL;DR is that we have people for whom remote execution will never be feasible, and so for this subset of people, we still want to make local builds as fast as possible, and that means precompiled headers which yield anywhere from 1.5x-2x speedup on builds.

I'll explain the semantics of how the build has to work, and I'm hoping someone can offer me some very specific pointers about where in the prelude I should be looking and what kind of interactions I need to be aware of. I'll also simplify the implementation a little so we don't have to worry about the "easy" parts. I'm also going to keep the discussion MSVC-centric because the difficult parts of the implementation shouldn't change much when talking about clang or some other compiler.

You have a `cxx_library` or `cxx_binary` (we'll call it `L`) whose cpp files are going to get compiled with some set `F` of compiler flags. Let's assume that for this library you don't have any file-specific arguments, so every file will get compiled exactly the same way. Before compiling any file in the library, you must _first_ compile a different file. Let's call it `pch.cpp`. It must have *exactly the same flags* as are in the set `F` that we talked about earlier, but add in one additional flag to tell the compiler to create a pch. (there's a little bit of flexibility here, and this isn't strictly true, but let's just say it is).

This produces two outputs, a `.obj` and a `.pch` file. Now, when compiling `L`, every cpp file must accept the `.pch` file on its command line using an option that tells the compiler to consume the PCH. Also, for the final link step, you need to include the `.obj` file.

Importantly, the output of the precompiled header build is an input to every individual compilation, meaning it's not enough to just have a cxx_library that I pass the "create" flag into, and have it be a dep of the cxx_library that consumes the PCH, because we need to guarnatee that the PCH is materialized first and is an input to the compile actions on the consumtion library.

I can think of several different options:

```
// option 1, hackish. no rule modifications, just stick the flags on.

_flags = ["/A", "/B", ...]

cxx_library(name="pch", srcs=["pch.cpp"], flags=_flags + ["/Yc"])
cxx_library(name="L", srcs=["a.cpp", "b.cpp"], flags=_flags + ["/Yu$(location :pch)"])
```

```
// option 2. first class rule knowledge

// take all of th same attrs as cxx_library(), and internally call _cxx_library_impl with the right args so that pch.cpp generates the pch output file.
cxx_precompiled_header(name="pch", src="pch.cpp")

// _cxx_library_impl is updated so that if the pch attribute is present, each compile action is extended with a flag to take the output of the pch target as an input to the compile action with appropriate flag.
cxx_library(name="L", srcs=["a.cpp", "b.cpp"], pch=":pch")
```

```
// option 3. anon target

// doesn't register any actions, all it does is return a CxxPrecompiledHeaderInfo provider with the a reference to the SourceArtifact for pch.cpp
cxx_precompiled_header(name="pch", src="pch.cpp")

// if pch attr is present, make an anon rule that invokes _compile_one_file forwarding all relevant attributes from the outer invocation, so that the pch compilation gets the same arguments.
cxx_library(name="L", srcs=["a.cpp", "b.cpp"], pch=":pch")
```

option 1 is the easiest, but I'm not even sure it works because of the string parameter macro. And I think the .obj has to be linked in the end too, which won't happen

option 2 probably works, but it's not ideal. You have to make sure that the consumer and producer were specified with exactly the same flags, and if you don't then you'll get weird compiler errors.

option 3 is the best, but I don't know how to make it work because it's my understanding you can't forward toolchain deps to an anon target, and the anon target is going to be compiling a file, so it effectively needs to recurse into _cxx_library_impl.

Would appreciate some detailed guidance

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.