Proposal: Stop recommending forward declaration in chromium C++ style guide

[Takuto Ikuta]

Hi,

While investigating possibilities of misc-include-cleaner check with clang-tidy in https://crbug.com/336474469, we found that clang's include cleaner doesn't work well with forward declarations as forward declaration has many disadvantages explained in

• http://go/fwd_decl_harmful
• http://go/totw/192
• https://google.github.io/styleguide/cppguide.html#Forward_Declarations
  
• https://crbug.com/336474469#comment19
  

I think these concerns are valid for chromium codebase. And I only found that Chromium recommends forward declaration from

https://codereview.chromium.org/2127713004/diff/20001/styleguide/c++/c++.md

as the oldest style guide update. But do we still want to choose compilation speed accepting these risks introduced by forward declaration in our style guide?

If not, to utilize clang's include cleaner, can we change our style guide to discourage forward declaration of class/struct and align with google style guide in that part?

Thanks,

Takuto

[Hans Wennborg]

> Do we still want to choose compilation speed accepting these risks introduced by forward declaration in our style guide?

For me, the answer is a definite yes. Build speed is a very important part of the developer experience, and while it's already too slow as it is, replacing forward declarations with full definitions in general would be a disaster for build speed.

There's a good reason Chromium has used forward declarations since the beginning, and also projects that it depended on like WebKit.

I think the internal style guide is wrong, but that it can work in an environment with virtually unlimited build resources. (It could also work in a C++20 Modules environment, but that seems some way out still.)

From http://go/totw/192:

> "The compile-time argument gets presented somewhat regularly, but that argument is often not valid (and even when it is, the impact is fairly minimal)." 

I believe that is measurably false. Include bloat is real, and reducing it by using forward declarations has substantial impact. For example, crbug.com/40103095 was a 4% reduction, crrev.com/870463 was 2%, and there have been many many others. See also Figma's post from earlier this year: https://www.figma.com/blog/speeding-up-build-times/

One way to experiment with this would be to replace includes of foo.mojom-forward.h with foo.mojom.h everywhere. Or to use full includes instead of forward decls in some central Blink headers (e.g. event_target.h) and see how that affects build speed.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALNjmMoWstkpntP%2B0xJC0wWitgYMVPJsvd7%3D8HKOPaQ7eiZoBw%40mail.gmail.com.

[mark a. foltz]

+1 to what Hans said; compilation speed is especially a factor for external contributors and researchers who don't have access to Google's build infrastructure.

m.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAB8jPhcgeMYBvrzPm2vtgWWgBkXOCYSEa0ZRgNC-Rg%3DYATEwLQ%40mail.gmail.com.

[K. Moon]

I think we should be careful to distinguish between build cost and build latency here, as they're not always the same thing in a parallel build environment.

Anyway, something I was chatting about with pkasting@ the other day is if we could ban "ad hoc" forward includes only. That is, forward includes could only be defined in a central header maintained by the provider of the symbol. This could help a lot with headers that are large/expensive, but not included everywhere anyway (like the standard library headers tend to be).

This might not help with the included cleaner (this was more about maintainability of widely distributed ad hoc forward declarations), although maybe if there were some sort of annotation mechanism like IWYU uses, it could work?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALgg%2BHGd6Xi%2BaQD3pe60tPnkrPE4hc1jR0%2BG0N2W1QbuT%2Ba7%3Dw%40mail.gmail.com.

[Peter Kasting]

On Tue, Aug 6, 2024 at 1:11 AM Hans Wennborg <ha...@chromium.org> wrote:

There's a good reason Chromium has used forward declarations since the beginning, and also projects that it depended on like WebKit.

Well... a lot of our decisions were based on existing google3 practice at the time (which has since changed), that we back-justified in plausible ways.

Chromium has never seriously experimented with a no-forward-decls policy. We don't know what the best possible world in both scenarios looks like.

And there are a lot of subtle ODR problems with templates that using forward decls can introduce. I would not be quick to dismiss the costs of those. Much like how component builds save a bit of time for a lot of people frequently, but then cost large chunks of time for a smaller number of people less frequently, the tradeoffs are not clear to me here.

I think the internal style guide is wrong, but that it can work in an environment with virtually unlimited build resources. (It could also work in a C++20 Modules environment, but that seems some way out still.)

FWIW, if full includes do add significant compile cost, jumbo ought to largely mitigate that (and it is very achievable in the near term, unlike modules). We do have choices here; we're generally just assuming we've already made the best ones.

Include bloat is real, and reducing it by using forward declarations has substantial impact.

Include bloat is absolutely real, but isn't the same problem as includes-vs.-forward-decls. In fact, by making tooling more tractable, banning forward decls can help us get to a place with less include bloat.

All this is not to say we should ban forward decls. I think it might be helpful here for anyone proposing changes to write an (informal) doc summarizing what we care about (correctness, build times, ease of refactoring, replacing manual action with tooling, simple rules, etc.) and trying to clarify how different choices we make affect those.

PK

[Daniel Cheng]

I would love to see us move away from forward declares :)

1. It really bugs me that core things like base::SupportsUserData have to use private implementations + forward declares. Does it speed up developer builds? Sure. Is it pretty much worse in every other possible way? Also yes :)

2. It makes it harder to explain things to developers who aren't familiar with "the way things work", e.g. for mojo headers.

3. It makes certain types of work harder, e.g. I'm trying to measure the binary size cost of not having the complex ctor warning, but I've had to add implementation complexity in the automated rewriter to avoid running into issues with forward declares.

Do C++20 modules have technical/specification issues that prevent us from using them? Do we have concrete numbers on whether using C++20 modules would help build times? If we know it would help build times, and the only reason we can't use it is that no one has prioritized implementing the glue... seems to me like we should be investing in that rather than leaning on forward declares.

Daniel

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFCKOgo-B%2BM5VApC3JOUhdk4zWQPY8OvRtj-DN3RsCCUHA%40mail.gmail.com.

[Nico Weber]

Strong +1 to what Hans says.

> Chromium has never seriously experimented with a no-forward-decls policy.

I'm not sure what kind of experiments this refers to, but we do have lots of data showing that forward declarations have big compile time effects. Hans linked to a bunch.

> Modules…

Sure, once that works, we can re-measure and reconsider. At the moment, we don't have them working yet.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF3XrKrwAgTOtUZHOCcjoET4KYkYr0izCX1ia5Qd-CcdYQf9gg%40mail.gmail.com.

[Peter Kasting]

On Tue, Aug 6, 2024 at 9:34 AM Nico Weber <tha...@chromium.org> wrote:

> Chromium has never seriously experimented with a no-forward-decls policy.

I'm not sure what kind of experiments this refers to, but we do have lots of data showing that forward declarations have big compile time effects. Hans linked to a bunch.

There are definitely forward decl changes in what Hans linked to, and also lots of non-forward decl changes (#include cleanups and the like).

What I was trying to say is that while we know that, in our current world, replacing specific heavyweight #includes with forward decls can reduce tokens (and in the aggregate thus lower build times), we don't know what other worlds are like, because we weren't ever in them. We didn't start with a "no forward decls" policy and then move away from it; we started here.

PK

[Nico Weber]

Not true. We used to not use forward headers for mojo-generated code for example, changed that to forward decls, and it was a big win.

As Hans says, it's easy to change the mojo compiler to put full declarations into the -forward files and re-measure what happens.

[Will Harris]

Just a data-point (not staking my opinion either way on forward declares debate), but forward declares also allow developers to very easily bypass gn check and DEPS rules as the tooling we have only check for .h file includes. This can introduce quite subtle layering violations that only become apparent later if/when the .h is included.

We could potentially fix this by adding tooling that verifies the right deps are present for forward declaring, but I'm not sure how complex that would be.

Will

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFCGZL3SZWLdwRdkQezU7wJjVL6zEFXipThiyW8nnrp_UQ%40mail.gmail.com.

[Peter Kasting]

On Tue, Aug 6, 2024 at 10:25 AM Nico Weber <tha...@chromium.org> wrote:

On Tue, Aug 6, 2024 at 12:52 PM Peter Kasting <pkas...@chromium.org> wrote:

What I was trying to say is that while we know that, in our current world, replacing specific heavyweight #includes with forward decls can reduce tokens (and in the aggregate thus lower build times), we don't know what other worlds are like, because we weren't ever in them. We didn't start with a "no forward decls" policy and then move away from it; we started here.

Not true. We used to not use forward headers for mojo-generated code for example, changed that to forward decls, and it was a big win.

That's coherent with what I'm saying. I don't disagree that forward-decls can make significant build time differences, compared to simply not having used them and not changing anything else.

Maybe the point I was attempting to make is not important, and that's why we're talking past each other. So I will reformulate it as: _if_ we wanted to try to get to a world with fewer of the hazards of forward decls, and also without paying too much build time penalty, I think that might be doable. These may not be mutually exclusive, at least not completely.

Knowing how big the hazards of forward-decls are, for us, would be useful, e.g. how often the kinds of problems Will mentions have happened, or how much tooling win could be unblocked with different amounts of paring them back.

PK

[Daniel Cheng]

Maybe it got missed, but I'm still wondering: does anyone know how much C++20 modules help with build times? Would they mitigate the costs of not using forward declarations?

Daniel

[Peter Kasting]

On Tue, Aug 6, 2024 at 11:15 AM Daniel Cheng <dch...@chromium.org> wrote:

does anyone know how much C++20 modules help with build times?

Not clear; the closest thing to Chromium that has released any numbers is the Microsoft Office transition to modules, but even there they're not actually done yet; they have been incrementally coalescing their build system. They also were using a heavily-optimized PCH setup extensively, so their starting point is not the same.

I have seen estimates for a net benefit for Chromium compile times anywhere from 10%-50% from today. Given that jumbo alone cuts CPU seconds for compile time by about 50% and modules are likely to be more effective overall, I would probably lean towards the high side of that range, but CPU seconds are not wall clock seconds.

Coincidentally, I was linked to https://github.com/ChuanqiXu9/clang-modules-converter today, which might be useful once we do want to move toward modules.

Would they mitigate the costs of not using forward declarations?

Yes.

PK

[Takuto Ikuta]

Thank you for your feedback and some references!

For now, it seems that we prefer to choose build performance rather than discouraging forward declaration and we also want to evaluate modules more for build times.

As far as I know, there is https://crbug.com/40440396 to use C++ modules in chromium.

So I changed my mind not to pursue clang's include cleaner until when it has support for forward declaration as we want.

Instead, I'll take a look at C++ modules and IWYU to see whether we can reduce build time more.

Thanks,

Takuto

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFAT6BnU_By-GA1zMGE%3DZdzoA2kwpCWzSkNeLh5MD-6fnw%40mail.gmail.com.

[Erik Staab]

On Tue, Aug 6, 2024 at 8:01 PM Takuto Ikuta <tik...@chromium.org> wrote:

Thank you for your feedback and some references!

For now, it seems that we prefer to choose build performance rather than discouraging forward declaration and we also want to evaluate modules more for build times.

As far as I know, there is https://crbug.com/40440396 to use C++ modules in chromium.

+1 to this conclusion. Yes, that's the most recent state of modules in chromium. Note that that issue is about clang header modules and not C++20 modules, which I think will give us a lot of the benefit and still be tractable (e.g. we need includes to continue to work so tools like clangd can be used).

 

So I changed my mind not to pursue clang's include cleaner until when it has support for forward declaration as we want.

Instead, I'll take a look at C++ modules and IWYU to see whether we can reduce build time more.

IWYU seems good to keep investigating if we can't get support for forward declarations in include-cleaner. I'm happy to discuss a modules strategy if you're interested in diving into that space.

Thanks,

Erik

 

Thanks,

Takuto

On Wed, Aug 7, 2024 at 3:23 AM Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Aug 6, 2024 at 11:15 AM Daniel Cheng <dch...@chromium.org> wrote:

does anyone know how much C++20 modules help with build times?

Not clear; the closest thing to Chromium that has released any numbers is the Microsoft Office transition to modules, but even there they're not actually done yet; they have been incrementally coalescing their build system. They also were using a heavily-optimized PCH setup extensively, so their starting point is not the same.

I have seen estimates for a net benefit for Chromium compile times anywhere from 10%-50% from today. Given that jumbo alone cuts CPU seconds for compile time by about 50% and modules are likely to be more effective overall, I would probably lean towards the high side of that range, but CPU seconds are not wall clock seconds.

Coincidentally, I was linked to https://github.com/ChuanqiXu9/clang-modules-converter today, which might be useful once we do want to move toward modules.

Would they mitigate the costs of not using forward declarations?

Yes.

PK

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFAT6BnU_By-GA1zMGE%3DZdzoA2kwpCWzSkNeLh5MD-6fnw%40mail.gmail.com.

--

You received this message because you are subscribed to a topic in the Google Groups "cxx" group.
To unsubscribe from this topic, visit https://groups.google.com/a/chromium.org/d/topic/cxx/hQWQFZxUhwg/unsubscribe.
To unsubscribe from this group and all its topics, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALNjmMr0OR34QQYH_nraNogK4eXyE7msJRk%3DJXB0cf%2BHTTmSrA%40mail.gmail.com.