Chrome #include Analysis and speeding up the build

766 views
Skip to first unread message

Hans Wennborg

unread,
Jun 30, 2021, 8:50:41 AM6/30/21
to chromium-dev
For those who haven't seen it yet, I would like to plug the Chrome #include Analysis dashboard [1].

The tool analyzes the graph formed by #include directives in the C++ source code and shows things such as: how many times a header is included directly and indirectly, the transitive size of each file, which headers contribute the most to the compiler input size, etc.

The "Per-Edge Analysis" [2] is especially useful, as it shows which edges in the graph are responsible for adding the most compiler input size. This can be used as a list of includes to consider for removal to reduce build times.

Old analyses are archived at [3].

We have been using this tool for a while to drive the removal of superfluous includes to speed up the build: https://crbug.com/242216 Thanks to that, the Chromium Build Time graph [4] is now trending down.

If you would like to help speed up the build, perhaps as a quick break from more complicated work, consider looking at [2] and see if you find any includes which seem unnecessary or could be replaced by forward declarations or other simple code changes. Edges at the top are often harder to remove (otherwise they'd be gone already), but there should be plenty of opportunities a bit further down the list -- or perhaps look in a corner of the codebase that you're particularly familiar with. Faster builds are better builds!

Thanks,
Hans

Ken Russell

unread,
Jun 30, 2021, 5:07:00 PM6/30/21
to ha...@chromium.org, chromium-dev
This is very cool! Thanks for filing (and even preemptively fixing!) one of the WebGL-related edges that was adding hundreds of megabytes to the build!

-Ken



--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAB8jPhfv%2Bm%2BnXszY2RWd0USvRizNLYadWoiXKA8pyiLz1B_gRA%40mail.gmail.com.

Scott Violet

unread,
Jun 30, 2021, 5:51:37 PM6/30/21
to k...@chromium.org, ha...@chromium.org, chromium-dev
Big +1! I *love* faster builds.

  -Scott

Peter Kasting

unread,
Jul 1, 2021, 12:22:56 AM7/1/21
to ha...@chromium.org, chromium-dev
I encourage other developers to take a stab at this.

I just spent one hour, peered through the list from around #100-150, and wrote a pair of patches that will hopefully remove about 1.7 GB of input from the build.  It's not difficult to do.

PK

Takuto Ikuta

unread,
Jul 1, 2021, 12:43:39 AM7/1/21
to pkas...@chromium.org, Hans Wennborg, chromium-dev
Do we have those links somewhere in our docs?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Christian Dullweber

unread,
Jul 1, 2021, 3:51:10 AM7/1/21
to tik...@chromium.org, pkas...@chromium.org, Hans Wennborg, chromium-dev
This is great :) Is there a good way to see the impact of a single CL on build size?


Hans Wennborg

unread,
Jul 1, 2021, 5:33:11 AM7/1/21
to Christian Dullweber, tik...@chromium.org, pkas...@chromium.org, chromium-dev
There is a way, but it's not as good as I'd like:

$ rm -rf out/foo && gn gen --args="show_includes=true symbol_level=0 use_goma=true" out/foo && autoninja -C out/foo -v chrome | tools/clang/scripts/analyze_includes.py -

That will print the total build size, but it takes ca 10 minutes to run and you'd need to run it before and after your CL.

What I would really like is something in Gerrit which could show the build size impact of a CL similarly to what we do for binary size on Android. Ideally it should warn about large increases and have a good way of showing where the increase comes from. If folks think that's a good idea, maybe I can work to make it happen.

Hans Wennborg

unread,
Jul 1, 2021, 5:35:37 AM7/1/21
to Takuto Ikuta, pkas...@chromium.org, chromium-dev

Bruce Dawson

unread,
Jul 2, 2021, 1:00:40 PM7/2/21
to Chromium-dev, Hans Wennborg, Peter Kasting, chromium-dev, tik...@chromium.org
The current include analysis is Linux focused, so it misses my "favorite" problematic include, <windows.h>. That has been tracked for a while under crbug.com/796644 and I've recently started doing some work on it. Reducing windows.h usage doesn't seem to improve build speeds much (but it does help a bit, of course) but it greatly reduces namespace pollution by reducing how many translation units are polluted with "#define DrawText DrawTextW" and friends.

In an includes analysis snapshot from a week or two ago windows.h was contributing 287 GB (!!!) to the size of the code being compiled. Recent changes have more than halved the number of translation units that include windows.h (when building the 'chrome' target), and the goal is to halve that again. Again, this doesn't improve build speeds much (I guess windows.h is easy to parse?) but it's still goodness.

Contact me if you want to collaborate on that bug.

Hans Wennborg

unread,
Jul 7, 2021, 3:36:10 PM7/7/21
to Peter Kasting, chromium-dev
I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=1227131 for those who want to contribute to this as part of the upcoming fixit.

Bruce Dawson

unread,
Jul 13, 2021, 1:53:14 PM7/13/21
to ha...@chromium.org, Peter Kasting, chromium-dev
Keep #include reductions in mind when writing and reviewing code also. Any #include that you add to a header file may end up being pulled in to hundreds or thousands of translation units. Avoiding this preemptively is easier than fixing it afterwards. Here is the relevant quote from the Chromium C++ style guide:

Unlike the Google style guide, Chromium style prefers forward declarations to #includes where possible. This can reduce compile times and result in fewer files needing recompilation when a header changes.

You can and should use forward declarations for most types passed or returned by value, reference, or pointer, or types stored as pointer members or in most STL containers. However, if it would otherwise make sense to use a type as a member by-value, don't convert it to a pointer just to be able to forward-declare the type.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to a topic in the Google Groups "Chromium-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/a/chromium.org/d/topic/chromium-dev/0ZME4DuE06k/unsubscribe.
To unsubscribe from this group and all its topics, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAB8jPhfnzpT0jVFzNX%3DdQ0RLCbpGe6c41meWM1DB%2BegndyrnFg%40mail.gmail.com.


--
Bruce Dawson, he/him

James Cook

unread,
Jul 13, 2021, 2:42:39 PM7/13/21
to Bruce Dawson, ha...@chromium.org, Peter Kasting, chromium-dev
On Tue, Jul 13, 2021 at 10:52 AM 'Bruce Dawson' via Chromium-dev <chromi...@chromium.org> wrote:
Keep #include reductions in mind when writing and reviewing code also. Any #include that you add to a header file may end up being pulled in to hundreds or thousands of translation units. Avoiding this preemptively is easier than fixing it afterwards. Here is the relevant quote from the Chromium C++ style guide:

Unlike the Google style guide, Chromium style prefers forward declarations to #includes where possible. This can reduce compile times and result in fewer files needing recompilation when a header changes.

You can and should use forward declarations for most types passed or returned by value, reference, or pointer, or types stored as pointer members or in most STL containers. However, if it would otherwise make sense to use a type as a member by-value, don't convert it to a pointer just to be able to forward-declare the type.

I wonder if we should relax the "don't convert it to a pointer just to be able to forward-declare the type" rule. Or just delete that entire sentence.

I frequently work in code where the CPU/memory cost of an extra unique_ptr<> is trivial (e.g. one extra heap allocation across the entire lifetime of chrome). Sometimes I'd rather have the compile-time benefit.
 

On Wed, Jul 7, 2021 at 12:36 PM Hans Wennborg <ha...@chromium.org> wrote:
I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=1227131 for those who want to contribute to this as part of the upcoming fixit.

On Thu, Jul 1, 2021 at 6:21 AM Peter Kasting <pkas...@chromium.org> wrote:
I encourage other developers to take a stab at this.

I just spent one hour, peered through the list from around #100-150, and wrote a pair of patches that will hopefully remove about 1.7 GB of input from the build.  It's not difficult to do.

PK

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to a topic in the Google Groups "Chromium-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/a/chromium.org/d/topic/chromium-dev/0ZME4DuE06k/unsubscribe.
To unsubscribe from this group and all its topics, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAB8jPhfnzpT0jVFzNX%3DdQ0RLCbpGe6c41meWM1DB%2BegndyrnFg%40mail.gmail.com.


--
Bruce Dawson, he/him

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAE5mQiOK3jTukHMSJCQD67LZi8BMEucrmRWQcWPOpZNb5XxGfA%40mail.gmail.com.

Hans Wennborg

unread,
Jul 27, 2021, 2:22:11 PM7/27/21
to Peter Kasting, chromium-dev
Many thanks to everyone who has been landing changes to improve this over the last couple of weeks!

From when this thread started (chromium.src c8791bef4136) to when I measured last night (5b43b47d7f60), we have removed almost 12 GB (5.13%) of compiler input size. If one imagines that as a stack of 1.44 MB 3.5" floppy disks, the stack would be 28 meters tall!

On my machine, that translates to a 128 CPU-minutes (6.1%) build time reduction (Linux, 'chrome' build target, default gn args, best of three), which I think is pretty great.

I'm really happy to see that so many wanted to contribute to this, and I hope we can keep the momentum going to push the build time even further down. If you have more ideas for improving the tools, process, etc. let me know.

Thanks,
Hans

Igor Bukanov

unread,
Aug 13, 2021, 8:17:22 AM8/13/21
to Chromium-dev, Hans Wennborg, chromium-dev, Peter Kasting
The efforts to reduce #include dependencies sounds similar to what removed Jumbo build option did except it was done automatically and was much more successful at reducing build time.  

Dirk Pranke

unread,
Aug 16, 2021, 5:27:17 PM8/16/21
to ig...@vivaldi.com, Chromium-dev, Hans Wennborg, Peter Kasting
Unfortunately, jumbo had the problems that it wasn't necessarily compatible with all standard-compliant C++ code, and it didn't benefit people that were using build accelerators like goma as much. #include reduction benefits everyone and is standards-compliant.

-- Dirk

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Igor Bukanov

unread,
Aug 17, 2021, 7:02:49 AM8/17/21
to Dirk Pranke, Chromium-dev, Hans Wennborg, Peter Kasting
On 2021-08-16 23:25, Dirk Pranke wrote:
> Unfortunately, jumbo had the problems that it wasn't necessarily
> compatible with all standard-compliant C++ code

Chromium sources use a dialect of C++ that is not even a subset of
standard-compliant C++, but a different language due to switching off
the exceptions. One cannot mix standard-compliant C++ and Chromium code.

Jumbo on other hand was standard-compliant as it required to program in
a subset of standard C++. That is, it required to ensure uniqueness of
names across C++ sources that were combined into single jumbo source.
And it was trivially possible to mix Jumbo and non-jumbo code by
disabling jumbofication in the build system of affected sources.

> #include
> reduction benefits everyone and is standards-compliant.

Just as with Jumbo strict adherence to include reduction requires to
program in a subset of C++. Namely, one has to avoid #include and
instead pre-declare types. This even reverses the Google C++ guide and
there were suggestions to use more unique_ptr in headers so more
#include can be avoided.

As for benefits consider that Jumbo made sources more grepable while
include reduction implies more efforts to find the header file to look
for the full definition of the class. Thanks for nice convention for
class names and file names this is typically not an issue in Chromium,
still it makes it harder to setup IDE tools so "Go to definition" works.

K. Moon

unread,
Aug 17, 2021, 12:20:14 PM8/17/21
to Igor Bukanov, Dirk Pranke, Chromium-dev, Hans Wennborg, Peter Kasting
Just wanted to correct something: It's not accurate to say that jumbo was standard-compliant C++, because it violated the standard's definition of a translation unit. It did so by preprocessing, but that's not the same thing as saying the compilation environment was the same in jumbo and non-jumbo nodes. (Otherwise, no jumbo-specific fixes would have been needed.)

I'm not exactly sure why we're rehashing this conversation again, though, because I don't see any chance that jumbo would come back. This seems off topic to this thread, which is about cleaning up excess #includes, which as Dirk notes, is helpful regardless.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
    http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Igor Bukanov

unread,
Aug 17, 2021, 2:14:04 PM8/17/21
to K. Moon, Dirk Pranke, Chromium-dev, Hans Wennborg, Peter Kasting
On 2021-08-17 18:18, K. Moon wrote:
> I'm not exactly sure why we're rehashing this conversation again,
> though, because I don't see any chance that jumbo would come back.
> This seems off topic to this thread, which is about cleaning up excess
> #includes, which as Dirk notes, is helpful regardless.

Replacing #include with class Foo deviates from Google C++ style guide.
As I understand the mayor justification for that is to save time with
local compilation. But Jumbo was much more effective at that (typical
reduction on a Linux was like 4-5 time faster to compile locally). So if
the local build time is problematic in some cases, why not to reconsider
the Jumbo support?

Note that efforts for include cleanups are very appreciated as they help
with local build time after small changes in the headers. But doing that
consistently creates a dialect of C++ that is harder to navigate and
explore, like the case of Qt headers where typically all data fields
lives in a private member class.

K. Moon

unread,
Aug 17, 2021, 2:36:17 PM8/17/21
to Igor Bukanov, Chromium-dev
I'm splitting off a new thread for this, since we're getting further off the original topic.

The ideal for Chromium builds is to use a distributed build system like Goma, so it's not really true that we want to optimize for local builds. Jumbo puts more work into fewer translation units, reducing total build costs, but this only reduces build latency if parallelism is limited. If parallelism is high, jumbo actually hurts build latency. It also increases the "blast radius" of changes, increasing how much of the build graph needs to be rebuilt for any given modification.

If a developer doesn't have access to a system like Goma (whether due to lack of resources, or lack of connectivity), of course that trade-off no longer works for them (although incremental builds may still be smaller). I think the bottom line is that we care more about Goma builds, though, so of course we're going to optimize as a project for those.

As for the forward declaration rule, that's a long-standing element of Chromium style that helps reduce the cost of compiling individual translation units, at the cost of having to maintain forward declarations manually. Whether for good or ill, that's the project-level decision, and consistency is the most important principle in the Google style guide.

Someday, modules might give us the best of all worlds, but that work seems like a long way off.

Igor Bukanov

unread,
Aug 17, 2021, 5:05:15 PM8/17/21
to km...@chromium.org, Chromium-dev
From outside perspective the resent burst in activities to reduce
include dependencies in Chromium including construction of tools to
facilitate that looks puzzling given that it replaces perfectly valid
code. Moreover, the replacement is not necessary neutral since it does
increase maintenance/reviewing cost and there are even suggestions to do
it even if it hurts performance metrics like using more of unique_ptr
for class fields.

And the only justification for doing this refactoring was that it
decreases build time. But with a sufficiently big build cluster this is
not observable. So the conclusion is that apparently saving local build
time without using Goma-like systems became important enough to allow
all these activities that essentially workaround a limitation of the
build system.

But then 2 years ago one of the argument against Jumbo was that it
required extra efforts to maintain workarounds for limitations of the
build system. So what changed during the last two years that now it is
OK to do such efforts? And if it is OK now, why not to consider Jumbo 2?
> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
> ---
> You received this message because you are subscribed to a topic in the
> Google Groups "Chromium-dev" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/a/chromium.org/d/topic/chromium-dev/0ZME4DuE06k/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> chromium-dev...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACwGi-6kooapTLLGqsGo0qJHpdZYg6se1Jg%2BM9aEF12W3Y%3DmXA%40mail.gmail.com
> [1].
>
>
> Links:
> ------
> [1]
> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACwGi-6kooapTLLGqsGo0qJHpdZYg6se1Jg%2BM9aEF12W3Y%3DmXA%40mail.gmail.com?utm_medium=email&utm_source=footer

Ken Rockot

unread,
Aug 17, 2021, 5:31:41 PM8/17/21
to ig...@vivaldi.com, km...@chromium.org, Chromium-dev
On Tue, Aug 17, 2021 at 2:04 PM Igor Bukanov <ig...@vivaldi.com> wrote:
 From outside perspective the resent burst in activities to reduce
include dependencies in Chromium including construction of tools to
facilitate that looks puzzling given that it replaces perfectly valid
code. Moreover, the replacement is not necessary neutral since it does
increase maintenance/reviewing cost and there are even suggestions to do
it even if it hurts performance metrics like using more of  unique_ptr
for class fields.

Build time reduction is not a reasonable justification for switching something to a heap allocation where performance might be a significant factor. Are we actually encouraging such changes somewhere?


And the only justification for doing this refactoring was that it
decreases build time. But with a sufficiently big build cluster this is
not observable. So the conclusion is that apparently saving local build
time without using Goma-like systems became important enough to allow
all these activities that essentially workaround a limitation of the
build system.

Are any large-scale refactorings happening as a result of this effort? IIUC the original post was about a dashboard which makes it easier to detect and eliminate redundant or otherwise problematic (large, viral) includes. I think it's clear that the project is concerned with local build times, it's just a question of trade-offs.


But then 2 years ago one of the argument against Jumbo was that it
required extra efforts to maintain workarounds for limitations of the
build system. So what changed during the last two years that now it is
OK to do such efforts? And if it is OK now, why not to consider Jumbo 2?

The build system was not the only issue. Jumbo builds imposed subtle constraints on how we named symbols, used namespaces, and generally organized our code. These incurred costs across engineering throughput and project maintenance. I think it's safe to say that if these costs were acceptable, we would still be supporting Jumbo builds. I personally ran into enough weird nuances that I was quite relieved when we stopped supporting them.

In contrast I don't really see how the existence of an #include analysis dashboard increases the cost of development or maintenance for anyone on the project, internal or external. It doesn't seem like a fair comparison.

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/8b12c5346f751f99794e7dbaf249e3ea%40vivaldi.com.

Dirk Pranke

unread,
Aug 17, 2021, 6:52:53 PM8/17/21
to ig...@vivaldi.com, km...@chromium.org, Chromium-dev
On Tue, Aug 17, 2021 at 2:04 PM Igor Bukanov <ig...@vivaldi.com> wrote:
 From outside perspective the resent burst in activities to reduce
include dependencies in Chromium including construction of tools to
facilitate that looks puzzling given that it replaces perfectly valid
code. Moreover, the replacement is not necessary neutral since it does
increase maintenance/reviewing cost and there are even suggestions to do
it even if it hurts performance metrics like using more of  unique_ptr
for class fields.

And the only justification for doing this refactoring was that it
decreases build time. But with a sufficiently big build cluster this is
not observable.

While it may be true that a sufficiently big build cluster can hide the cost of growth, it doesn't mean that it's free :), and we do care about saving costs as well as build times. 

Also, there are critical paths through the build that a build cluster can't necessarily speed up, but improving individual TU compilation times can. We have seen those benefits in the past as a result of similar work on include reductions (e.g., in speeding up blink compiles), although I wouldn't want to try and point to any recent examples of this.
 
So the conclusion is that apparently saving local build
time without using Goma-like systems became important enough to allow
all these activities that essentially workaround a limitation of the
build system.

As I note above, that's not the conclusion you should draw.
 
But then 2 years ago one of the argument against Jumbo was that it
required extra efforts to maintain workarounds for limitations of the
build system. So what changed during the last two years that now it is
OK to do such efforts? And if it is OK now, why not to consider Jumbo 2?

None of the arguments against the jumbo restrictions have gone away. If someone has an approach for something jumbo-like but that does not have jumbo's downsides, we're certainly interested.

-- Dirk
 
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/8b12c5346f751f99794e7dbaf249e3ea%40vivaldi.com.

Peter Kasting

unread,
Aug 17, 2021, 10:18:56 PM8/17/21
to ig...@vivaldi.com, km...@chromium.org, Chromium-dev
On Tue, Aug 17, 2021 at 2:03 PM Igor Bukanov <ig...@vivaldi.com> wrote:
 From outside perspective the resent burst in activities to reduce
include dependencies in Chromium including construction of tools to
facilitate that looks puzzling given that it replaces perfectly valid
code.

In general the quest here is to do proper "include what you use", in which case the code was not always technically valid before.

Moreover, the replacement is not necessary neutral since it does
increase maintenance/reviewing cost

IWYU should decrease maintenance cost, not increase it.  In other cases, this work has simply split headers into multiple pieces, which is generally neutral from a reviewing perspective.  I don't think I have seen work here that seems like it significantly hurts maintenance and readability.

there are even suggestions to do
it even if it hurts performance metrics like using more of  unique_ptr
for class fields.

Please point any reviewers who recommend this at cxx@, as this advice is generally contra-style guide and good practice, and we'd like to understand if people are recommending it.

And the only justification for doing this refactoring was that it
decreases build time.

On the contrary, I view the lower maintenance costs (in fixing missing #includes during refactoring) as being perhaps an even bigger benefit :).
 
But with a sufficiently big build cluster this is
not observable.

Oh?  Are you saying "I have a big build cluster and over period X where your dashboard went down by Y%, the build time remained static/increased"?
 
Because I think the contention from the build time side here is that we should ultimately observe this on cluster builds, not just on local builds.

PK

Igor Bukanov

unread,
Aug 18, 2021, 4:04:45 AM8/18/21
to pkas...@chromium.org, km...@chromium.org, Chromium-dev
On 2021-08-18 04:17, Peter Kasting wrote:
> IWYU should decrease maintenance cost, not increase it. In other
> cases, this work has simply split headers into multiple pieces, which
> is generally neutral from a reviewing perspective. I don't think I
> have seen work here that seems like it significantly hurts maintenance
> and readability.

The rule of using #include when one needs to refer to a class Foo in any
form is very easy to follow. The rule of writing "class Foo;" is less so
since it depends on the usage of Foo. Plus there is a reviewing cost as
the reviewer should look if an extra #include in the header to see if it
is justified. This cost can be trivial, but it is not zero.

> Please point any reviewers who recommend this at cxx@, as this advice
> is generally contra-style guide and good practice, and we'd like to
> understand if people are recommending it.

See the suggestion in
https://groups.google.com/a/chromium.org/g/chromium-dev/c/0ZME4DuE06k/m/GmrCzZGDBwAJ

Igor Bukanov

unread,
Aug 18, 2021, 4:21:35 AM8/18/21
to Dirk Pranke, km...@chromium.org, Chromium-dev
On 2021-08-18 00:51, Dirk Pranke wrote:
> None of the arguments against the jumbo restrictions have gone away.
> If someone has an approach for something jumbo-like but that does not
> have jumbo's downsides, we're certainly interested.

IMO the biggest problem with jumbo was that it was add-on that people
was not aware on until the problem arisen on some test build with hard
to understand compilation errors.

Yet as SQLight with its option to turn the whole code into one source
file shows there are non-trivial performance benefits of combing several
sources into one compilation unit. The compiler can simply optimise more
as not every optimisation can be postponed to the link time. So an
explicit jumbofication of sources done primary with performance as a
primary goal (with compilation time decrease as a side benefit) can be a
better option, compared with an implicit, non-default and pretty random
combining of sources as was done with the original jumbo.

K. Moon

unread,
Aug 19, 2021, 11:43:56 AM8/19/21
to Igor Bukanov, Dirk Pranke, Chromium-dev
At the end of the day, these decisions need to be data-driven, so there's not a whole lot of point to theorizing about the potential performance benefits without measuring. (Also, I don't think we ever shipped jumbo builds to end users?)

Before wasting time on that, though, I have to ask what the end goal is here? Jumbo isn't coming back. At best, I think it'd make sense as a set of externally-maintained patches. That would at least put the maintenance cost on those who actually use it, rather than spreading it to everyone else who doesn't.

Fergal Daly

unread,
Nov 9, 2021, 1:41:47 AM11/9/21
to ha...@chromium.org, chromium-dev
On Wed, 30 Jun 2021 at 21:49, Hans Wennborg <ha...@chromium.org> wrote:
For those who haven't seen it yet, I would like to plug the Chrome #include Analysis dashboard [1].

The tool analyzes the graph formed by #include directives in the C++ source code and shows things such as: how many times a header is included directly and indirectly, the transitive size of each file, which headers contribute the most to the compiler input size, etc.

The "Per-Edge Analysis" [2] is especially useful, as it shows which edges in the graph are responsible for adding the most compiler input size. This can be used as a list of includes to consider for removal to reduce build times.

Old analyses are archived at [3].

We have been using this tool for a while to drive the removal of superfluous includes to speed up the build: https://crbug.com/242216 Thanks to that, the Chromium Build Time graph [4] is now trending down.

If you would like to help speed up the build, perhaps as a quick break from more complicated work, consider looking at [2] and see if you find any includes which seem unnecessary or could be replaced by forward declarations or other simple code changes. Edges at the top are often harder to remove (otherwise they'd be gone already), but there should be plenty of opportunities a bit further down the list -- or perhaps look in a corner of the codebase that you're particularly familiar with. Faster builds are better builds!

Is there any good way to measure the impact of a change?

F
 
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Takuto Ikuta

unread,
Nov 9, 2021, 2:03:39 AM11/9/21
to fer...@google.com, ha...@chromium.org, chromium-dev
On Tue, Nov 9, 2021 at 3:40 PM 'Fergal Daly' via Chromium-dev <chromi...@chromium.org> wrote:
On Wed, 30 Jun 2021 at 21:49, Hans Wennborg <ha...@chromium.org> wrote:
For those who haven't seen it yet, I would like to plug the Chrome #include Analysis dashboard [1].

The tool analyzes the graph formed by #include directives in the C++ source code and shows things such as: how many times a header is included directly and indirectly, the transitive size of each file, which headers contribute the most to the compiler input size, etc.

The "Per-Edge Analysis" [2] is especially useful, as it shows which edges in the graph are responsible for adding the most compiler input size. This can be used as a list of includes to consider for removal to reduce build times.

Old analyses are archived at [3].

We have been using this tool for a while to drive the removal of superfluous includes to speed up the build: https://crbug.com/242216 Thanks to that, the Chromium Build Time graph [4] is now trending down.

If you would like to help speed up the build, perhaps as a quick break from more complicated work, consider looking at [2] and see if you find any includes which seem unnecessary or could be replaced by forward declarations or other simple code changes. Edges at the top are often harder to remove (otherwise they'd be gone already), but there should be plenty of opportunities a bit further down the list -- or perhaps look in a corner of the codebase that you're particularly familiar with. Faster builds are better builds!

Is there any good way to measure the impact of a change?


I guess it is difficult to get visible improvement for a clean build time unless you made a CL having a very large cleanup or removed header is exceptionally large.

But maybe we can see somewhat visible improvement in incremental build.
e.g. 
* If we remove A.h from B.cc, we can remove compile time for B.o when we touch A.h.
* Or if we remove A.h from B.h, we see less number of compiles when we touch A.h for some build targets.
 

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Hans Wennborg

unread,
Nov 9, 2021, 7:19:53 AM11/9/21
to Fergal Daly, chromium-dev
On Tue, Nov 9, 2021 at 7:40 AM Fergal Daly <fer...@google.com> wrote:
>
> On Wed, 30 Jun 2021 at 21:49, Hans Wennborg <ha...@chromium.org> wrote:
>>
>> For those who haven't seen it yet, I would like to plug the Chrome #include Analysis dashboard [1].
>>
>> The tool analyzes the graph formed by #include directives in the C++ source code and shows things such as: how many times a header is included directly and indirectly, the transitive size of each file, which headers contribute the most to the compiler input size, etc.
>>
>> The "Per-Edge Analysis" [2] is especially useful, as it shows which edges in the graph are responsible for adding the most compiler input size. This can be used as a list of includes to consider for removal to reduce build times.
>>
>> Old analyses are archived at [3].
>>
>> We have been using this tool for a while to drive the removal of superfluous includes to speed up the build: https://crbug.com/242216 Thanks to that, the Chromium Build Time graph [4] is now trending down.
>>
>> If you would like to help speed up the build, perhaps as a quick break from more complicated work, consider looking at [2] and see if you find any includes which seem unnecessary or could be replaced by forward declarations or other simple code changes. Edges at the top are often harder to remove (otherwise they'd be gone already), but there should be plenty of opportunities a bit further down the list -- or perhaps look in a corner of the codebase that you're particularly familiar with. Faster builds are better builds!
>
>
> Is there any good way to measure the impact of a change?

Yes, you can run this:

$ rm -rf out/Debug && gn gen --args="show_includes=true symbol_level=0
use_goma=true" out/Debug && autoninja -C out/Debug -v chrome |
tools/clang/scripts/analyze_includes.py -

before and after the change to measure the total build sizes. It takes
5-ish minutes depending on the machine.

I'm hoping to make that faster when I find the time. Also it would be
very nice to be able to diff two builds and see /where/ the changes
come from. The ultimate goal would then be to surface that on Gerrit
similarly to what we have for Android binary size.
(crbug.com/1229609).

Thanks,
Hans
Reply all
Reply to author
Forward
0 new messages