Re: Improvement to build times through cleanup of C++ include dependencies
Hello,
Le 15/12/2020 à 10:36, Gabriele Svelto a écrit :
Thanks for this work Simon, this is awesome!
There's also plenty of side effects to this that will make life better
for developers, just a few off the top of my mind:
- All our static analysis passes need to go through the preprocessed
sources too, so less includes means less code to churn through when we
run them.
Gabriele, you made me look to our instance of Codechecker [1] that run most of
clang-{tidy, analyzer} checkers.
It dropped from ~17h on average to 14h30m for a full analysis!
Well done!
Thanks
Sylvestre
[1] https://github.com/Ericsson/codechecker/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Improvement to build times through cleanup of C++ include dependencies
Hi Chris, I am not 100% sure if that's feasible with a near-zero rate of false positives, but it definitely seems worth trying out. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1682477 to address this. Simon On Mon, Dec 14, 2020 at 8:00 PM Chris Peterson wrote: > On 12/14/2020 3:23 AM, Simon Giesecke wrote: > > I was using some tools to support this, notably ClangBuildAnalyzer [2] > and > > include-what-you-use [3]. ClangBuildAnalyzer helped to detect headers > that > > are expensive to parse throughout the build, and direct efforts to reduce > > those specifically. But there remains a long tail of similar things that > > can and should be fixed. include-what-you-use helps to identify the > headers > > from which a file uses declarations to avoid depending on indirectly > > included files which might disappear from the include chain by unrelated > > changes. These tools are not readymade for everyday use, but I will try > to > > provide some ideas for using them effectively later on. > > Can include-what-you-use be configured as a Phabricator linter to warn > about new unused header includes before they are checked in? > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Improvement to build times through cleanup of C++ include dependencies
Thanks for this work Simon, this is awesome! There's also plenty of side effects to this that will make life better for developers, just a few off the top of my mind: - When sccache is in use files are preprocessed before being sent to sccache, so even when hitting the cache we pay the price of running the preprocessor so less includes will speed up sccache hot builds! - All our static analysis passes need to go through the preprocessed sources too, so less includes means less code to churn through when we run them. - When templates are involved the linker needs to de-duplicate identical instantiations in different compilation units, with less of them linking will be faster too. - And also less I/O which might seem academic if you've got a very fast SSD but not everybody has one (think about contributors who often don't have access to beefy boxes). Gabriele On 14/12/20 12:23, Simon Giesecke wrote: > tl;dr Build times on all platforms, in particular for incremental builds, > have decreased in the last weeks by landing several cleanups to C++ include > dependencies that reduce the aggregated number of included files by about > 30%. > > Hi, > > Did you notice a reduction in build times lately? This might not be a > coincidence. In this mail, I want to provide some details on the > improvements made. I want to thank everyone who contributed to this through > up-front discussions and reviews. > > Recently I landed a number of patches on Bug 1676346 [1] that in various > ways clean up C++ include directives in our codebase. Some landed ca. 3 > weeks ago, some landed last week. Overall, these reduce the aggregated > number of included non-system header files in a full build by about 30% on > Linux. I don't have numbers for other platforms, but they should benefit as > well. On my machine, this reduced the time for a clobber build by about > 10%. While that might go unnoticed, incremental builds are often sped up > even a lot more, since the number of translation units that need to be > rebuilt decreases. E.g. the number of translation units that include > dom/base/Document.h reduced by ca. 52%, resulting in a build time reduction > of 48% on my machine after modifying that. Your mileage may vary. > > While this might not spare you from buying new hardware, it will make > builds faster regardless of the hardware you are running on, and hopefully > increase productivity. If you want to share your experiences with me, > please get in touch! > > You might be curious what I did to achieve that, or how you can contribute > to reducing build times yourself. It's a combination of things, most > importantly three things: > 1. Remove unused include directives > 2. Split some headers > 3. Use forward declarations more > 4. Hide definitions to allow using forward declarations even more > > About 1: I found there are several include directives that are not needed > at all. They could simply be removed. However, the majority of cases were a > bit more complex, because of a lot of missing include directives. When > removing an include for X.h from a header file Y.h that doesn't need it, > another file that included Y.h might need X.h. Or, Y.h itself might need > something from a header indirectly included from X.h. Or similar cases. > This meant quite a lot of more include directives for more basic things > needed to be added to ensure the build doesn't break. > > About 2: Some headers have a lot of dependencies, but only relatively few > users of that header need them. One example was IPCMessageUtils.h, which is > included by all files generated by IPDL, which also contained a lot of > specializations of the ParamTraits template that are needed only by few > files. Apart from some very basic specializations, these were moved to the > new EnumSerializers.h and IPCMessageUtilsSpecializations.h as well as some > existing headers, so that the remaining IPCMessageUtils.h header has only > much more limited dependencies. > > About 3: Our coding style favors forward declarations over inclusion of the > full definition of types where possible. I replaced the inclusion of header > files containing full definitions of types by forward declarations at a > number of places where this is sufficient, e.g. because they are only used > in function signatures. It's worth noting that there were also a number of > cases where a forward declaration was present, but actually the full > definition is required, e.g. when a type is used as a base class or as the > value type of a data member, or an inline function body dereferences into a > member. > > About 4: As mentioned in the last point, inline function bodies often > require the inclusion of additional headers because they dereference into > types of which otherwise only forward declarations were necessary. Similar > considerations apply to private (nested) classes. Some of those were moved > to the corresponding implementation files to hide these dependencies, and > reduce th
Re: Improvement to build times through cleanup of C++ include dependencies
Due to the noisy nature of build times it might be too early to tell. Here's the telemetry dashboard for build: https://sql.telemetry.mozilla.org/dashboard/build?p_date=d_last_60_days (sorry - Mozilla employees only). Mike Conley wrote on 15/12/20 12:18 am: Thank you so much for investing time and effort into this area! Improvements to build times are always always welcome. I seem to recall we collect opt-in Telemetry on things like build times. If so, have we noticed any changes in those graphs? -Mike -- glob — engineering workflow manager — moz://a ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Improvement to build times through cleanup of C++ include dependencies
On 12/14/2020 3:23 AM, Simon Giesecke wrote: I was using some tools to support this, notably ClangBuildAnalyzer [2] and include-what-you-use [3]. ClangBuildAnalyzer helped to detect headers that are expensive to parse throughout the build, and direct efforts to reduce those specifically. But there remains a long tail of similar things that can and should be fixed. include-what-you-use helps to identify the headers from which a file uses declarations to avoid depending on indirectly included files which might disappear from the include chain by unrelated changes. These tools are not readymade for everyday use, but I will try to provide some ideas for using them effectively later on. Can include-what-you-use be configured as a Phabricator linter to warn about new unused header includes before they are checked in? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Improvement to build times through cleanup of C++ include dependencies
Thank you so much for investing time and effort into this area! Improvements to build times are always always welcome. I seem to recall we collect opt-in Telemetry on things like build times. If so, have we noticed any changes in those graphs? -Mike On Mon, 14 Dec 2020 at 11:11, Botond Ballo wrote: > Hey Simon, > > Thanks a lot for working on this! It's hard to notice the effect of > individual changes in this area, but in aggregate they can add up to a > significant build speed improvement. > > Botond > > On Mon, Dec 14, 2020 at 6:23 AM Simon Giesecke > wrote: > > > > tl;dr Build times on all platforms, in particular for incremental > builds, > > have decreased in the last weeks by landing several cleanups to C++ > include > > dependencies that reduce the aggregated number of included files by about > > 30%. > > > > Hi, > > > > Did you notice a reduction in build times lately? This might not be a > > coincidence. In this mail, I want to provide some details on the > > improvements made. I want to thank everyone who contributed to this > through > > up-front discussions and reviews. > > > > Recently I landed a number of patches on Bug 1676346 [1] that in various > > ways clean up C++ include directives in our codebase. Some landed ca. 3 > > weeks ago, some landed last week. Overall, these reduce the aggregated > > number of included non-system header files in a full build by about 30% > on > > Linux. I don't have numbers for other platforms, but they should benefit > as > > well. On my machine, this reduced the time for a clobber build by about > > 10%. While that might go unnoticed, incremental builds are often sped up > > even a lot more, since the number of translation units that need to be > > rebuilt decreases. E.g. the number of translation units that include > > dom/base/Document.h reduced by ca. 52%, resulting in a build time > reduction > > of 48% on my machine after modifying that. Your mileage may vary. > > > > While this might not spare you from buying new hardware, it will make > > builds faster regardless of the hardware you are running on, and > hopefully > > increase productivity. If you want to share your experiences with me, > > please get in touch! > > > > You might be curious what I did to achieve that, or how you can > contribute > > to reducing build times yourself. It's a combination of things, most > > importantly three things: > > 1. Remove unused include directives > > 2. Split some headers > > 3. Use forward declarations more > > 4. Hide definitions to allow using forward declarations even more > > > > About 1: I found there are several include directives that are not needed > > at all. They could simply be removed. However, the majority of cases > were a > > bit more complex, because of a lot of missing include directives. When > > removing an include for X.h from a header file Y.h that doesn't need it, > > another file that included Y.h might need X.h. Or, Y.h itself might need > > something from a header indirectly included from X.h. Or similar cases. > > This meant quite a lot of more include directives for more basic things > > needed to be added to ensure the build doesn't break. > > > > About 2: Some headers have a lot of dependencies, but only relatively few > > users of that header need them. One example was IPCMessageUtils.h, which > is > > included by all files generated by IPDL, which also contained a lot of > > specializations of the ParamTraits template that are needed only by few > > files. Apart from some very basic specializations, these were moved to > the > > new EnumSerializers.h and IPCMessageUtilsSpecializations.h as well as > some > > existing headers, so that the remaining IPCMessageUtils.h header has only > > much more limited dependencies. > > > > About 3: Our coding style favors forward declarations over inclusion of > the > > full definition of types where possible. I replaced the inclusion of > header > > files containing full definitions of types by forward declarations at a > > number of places where this is sufficient, e.g. because they are only > used > > in function signatures. It's worth noting that there were also a number > of > > cases where a forward declaration was present, but actually the full > > definition is required, e.g. when a type is used as a base class or as > the > > value type of a data member, or an inline function body dereferences > into a > > member. > > > > About 4: As mentioned in the last point, inline function bodies often > > require the inclusion of additional headers because they dereference into > > types of which otherwise only forward declarations were necessary. > Similar > > considerations apply to private (nested) classes. Some of those were > moved > > to the corresponding implementation files to hide these dependencies, and > > reduce the number of necessary includes in the header files. > > > > I was using some tools to support this, notably ClangBuildAnalyzer [2] > and > > include-what-you-use [3]. C
Re: Improvement to build times through cleanup of C++ include dependencies
Hey Simon, Thanks a lot for working on this! It's hard to notice the effect of individual changes in this area, but in aggregate they can add up to a significant build speed improvement. Botond On Mon, Dec 14, 2020 at 6:23 AM Simon Giesecke wrote: > > tl;dr Build times on all platforms, in particular for incremental builds, > have decreased in the last weeks by landing several cleanups to C++ include > dependencies that reduce the aggregated number of included files by about > 30%. > > Hi, > > Did you notice a reduction in build times lately? This might not be a > coincidence. In this mail, I want to provide some details on the > improvements made. I want to thank everyone who contributed to this through > up-front discussions and reviews. > > Recently I landed a number of patches on Bug 1676346 [1] that in various > ways clean up C++ include directives in our codebase. Some landed ca. 3 > weeks ago, some landed last week. Overall, these reduce the aggregated > number of included non-system header files in a full build by about 30% on > Linux. I don't have numbers for other platforms, but they should benefit as > well. On my machine, this reduced the time for a clobber build by about > 10%. While that might go unnoticed, incremental builds are often sped up > even a lot more, since the number of translation units that need to be > rebuilt decreases. E.g. the number of translation units that include > dom/base/Document.h reduced by ca. 52%, resulting in a build time reduction > of 48% on my machine after modifying that. Your mileage may vary. > > While this might not spare you from buying new hardware, it will make > builds faster regardless of the hardware you are running on, and hopefully > increase productivity. If you want to share your experiences with me, > please get in touch! > > You might be curious what I did to achieve that, or how you can contribute > to reducing build times yourself. It's a combination of things, most > importantly three things: > 1. Remove unused include directives > 2. Split some headers > 3. Use forward declarations more > 4. Hide definitions to allow using forward declarations even more > > About 1: I found there are several include directives that are not needed > at all. They could simply be removed. However, the majority of cases were a > bit more complex, because of a lot of missing include directives. When > removing an include for X.h from a header file Y.h that doesn't need it, > another file that included Y.h might need X.h. Or, Y.h itself might need > something from a header indirectly included from X.h. Or similar cases. > This meant quite a lot of more include directives for more basic things > needed to be added to ensure the build doesn't break. > > About 2: Some headers have a lot of dependencies, but only relatively few > users of that header need them. One example was IPCMessageUtils.h, which is > included by all files generated by IPDL, which also contained a lot of > specializations of the ParamTraits template that are needed only by few > files. Apart from some very basic specializations, these were moved to the > new EnumSerializers.h and IPCMessageUtilsSpecializations.h as well as some > existing headers, so that the remaining IPCMessageUtils.h header has only > much more limited dependencies. > > About 3: Our coding style favors forward declarations over inclusion of the > full definition of types where possible. I replaced the inclusion of header > files containing full definitions of types by forward declarations at a > number of places where this is sufficient, e.g. because they are only used > in function signatures. It's worth noting that there were also a number of > cases where a forward declaration was present, but actually the full > definition is required, e.g. when a type is used as a base class or as the > value type of a data member, or an inline function body dereferences into a > member. > > About 4: As mentioned in the last point, inline function bodies often > require the inclusion of additional headers because they dereference into > types of which otherwise only forward declarations were necessary. Similar > considerations apply to private (nested) classes. Some of those were moved > to the corresponding implementation files to hide these dependencies, and > reduce the number of necessary includes in the header files. > > I was using some tools to support this, notably ClangBuildAnalyzer [2] and > include-what-you-use [3]. ClangBuildAnalyzer helped to detect headers that > are expensive to parse throughout the build, and direct efforts to reduce > those specifically. But there remains a long tail of similar things that > can and should be fixed. include-what-you-use helps to identify the headers > from which a file uses declarations to avoid depending on indirectly > included files which might disappear from the include chain by unrelated > changes. These tools are not readymade for everyday use, but I will try to > provide some
Improvement to build times through cleanup of C++ include dependencies
tl;dr Build times on all platforms, in particular for incremental builds, have decreased in the last weeks by landing several cleanups to C++ include dependencies that reduce the aggregated number of included files by about 30%. Hi, Did you notice a reduction in build times lately? This might not be a coincidence. In this mail, I want to provide some details on the improvements made. I want to thank everyone who contributed to this through up-front discussions and reviews. Recently I landed a number of patches on Bug 1676346 [1] that in various ways clean up C++ include directives in our codebase. Some landed ca. 3 weeks ago, some landed last week. Overall, these reduce the aggregated number of included non-system header files in a full build by about 30% on Linux. I don't have numbers for other platforms, but they should benefit as well. On my machine, this reduced the time for a clobber build by about 10%. While that might go unnoticed, incremental builds are often sped up even a lot more, since the number of translation units that need to be rebuilt decreases. E.g. the number of translation units that include dom/base/Document.h reduced by ca. 52%, resulting in a build time reduction of 48% on my machine after modifying that. Your mileage may vary. While this might not spare you from buying new hardware, it will make builds faster regardless of the hardware you are running on, and hopefully increase productivity. If you want to share your experiences with me, please get in touch! You might be curious what I did to achieve that, or how you can contribute to reducing build times yourself. It's a combination of things, most importantly three things: 1. Remove unused include directives 2. Split some headers 3. Use forward declarations more 4. Hide definitions to allow using forward declarations even more About 1: I found there are several include directives that are not needed at all. They could simply be removed. However, the majority of cases were a bit more complex, because of a lot of missing include directives. When removing an include for X.h from a header file Y.h that doesn't need it, another file that included Y.h might need X.h. Or, Y.h itself might need something from a header indirectly included from X.h. Or similar cases. This meant quite a lot of more include directives for more basic things needed to be added to ensure the build doesn't break. About 2: Some headers have a lot of dependencies, but only relatively few users of that header need them. One example was IPCMessageUtils.h, which is included by all files generated by IPDL, which also contained a lot of specializations of the ParamTraits template that are needed only by few files. Apart from some very basic specializations, these were moved to the new EnumSerializers.h and IPCMessageUtilsSpecializations.h as well as some existing headers, so that the remaining IPCMessageUtils.h header has only much more limited dependencies. About 3: Our coding style favors forward declarations over inclusion of the full definition of types where possible. I replaced the inclusion of header files containing full definitions of types by forward declarations at a number of places where this is sufficient, e.g. because they are only used in function signatures. It's worth noting that there were also a number of cases where a forward declaration was present, but actually the full definition is required, e.g. when a type is used as a base class or as the value type of a data member, or an inline function body dereferences into a member. About 4: As mentioned in the last point, inline function bodies often require the inclusion of additional headers because they dereference into types of which otherwise only forward declarations were necessary. Similar considerations apply to private (nested) classes. Some of those were moved to the corresponding implementation files to hide these dependencies, and reduce the number of necessary includes in the header files. I was using some tools to support this, notably ClangBuildAnalyzer [2] and include-what-you-use [3]. ClangBuildAnalyzer helped to detect headers that are expensive to parse throughout the build, and direct efforts to reduce those specifically. But there remains a long tail of similar things that can and should be fixed. include-what-you-use helps to identify the headers from which a file uses declarations to avoid depending on indirectly included files which might disappear from the include chain by unrelated changes. These tools are not readymade for everyday use, but I will try to provide some ideas for using them effectively later on. Best wishes Simon [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1676346 [2] https://github.com/aras-p/ClangBuildAnalyzer [3] https://github.com/include-what-you-use/include-what-you-use ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
