Paths

Table of Contentst

[msvc/freestyle] Use precompiled headers for freestyle
ClosedPublic
Actions

Authored by Ray Molenkamp (LazyDodo) on Apr 7 2017, 9:06 PM.

Details

Reviewers

Sergey Sharybin (sergey)
Bastien Montagne (mont29)
Campbell Barton (campbellbarton)
Brecht Van Lommel (brecht)

Commits

rBS9c76b2c3b685: Freestyle: Use precompiled headers with MSVC.
rB9c76b2c3b685: Freestyle: Use precompiled headers with MSVC.

Summary

Freestyle is in the top 5 slow building projects (in both release and debug builds) with msvc.

While the code isn't overly complicated or hard to optimize for, there is *A LOT* of it nearly 250 compilation units, all including +- the same set of headers (most of them include python.h which will will be hard to get rid of if we rather just clean up the includes), making bf_freestyle a prime candidate for using pre-compiled headers.

This patch add a blender_precomp macro for specifying a precompiler header+cpp for any given project, with currently only an msvc implementation (i really have no idea how gcc handles this) speeding up the build process 3-5x (depending on compiler/configuration)

Results:

I'd classify this one as a WIP, as there are a couple of open questions

are we even willing to consider using precompiled headers?
should there be a global option for turning this on/off?
if this is a route we want to take, we probably need to expand the macro with some gcc support?

Another option to speed up compilation is trying to merge as many of the .cpp files as we can into a single compilation unit, (also known as a unity build) but imho that's a nightmare for the maintainer

a final option would be: do nothing, although it's slow, it's not *that* slow.

Diff Detail

Repository: rB Blender

Event Timeline

Ray Molenkamp (LazyDodo) created this revision.Apr 7 2017, 9:06 PM

Ray Molenkamp (LazyDodo) mentioned this in D2724: [Modifiers] Screw modifier generates doubles. .Jul 26 2017, 4:31 PM

If it takes over 2 years to have an opinion on this, it's not worth doing.

Actually, this *IS* still worth doing, build time in ms

vs2019 bf_freestyle debug   before: 60464 ms  after: 11028 ms
vs2019 bf_freestyle release before: 56984 ms  after: 20526 ms

Harbormaster completed remote builds in B3835: Diff 15804.Jun 5 2019, 5:19 PM

Ray Molenkamp (LazyDodo) added a reviewer: Brecht Van Lommel (brecht).Jun 5 2019, 5:19 PM

I guess most of the time comes from all those system headers, and not so much the BPy_* headers?

Is there a way to precompile those system headers for the entire project, instead of Freestyle specifically?

when excluding the intern/ headers

Debug    22705ms
Release  34760ms

I don't think you can have multiple precompiled headers per project so doing this globally would probably be troublesome.

Also when looking at this in the past, PCH speeds things up if lots and lots of compile units all include the same files

judging from a treemap from todays build times

I'd probably want to look into the compositor, but that's probably the only other one where gains could possibly be had.

Also i'd love to kick ceres to svn, but that's a conversation for a different diff i suppose

Also i'd love to kick ceres to svn, but that's a conversation for a different diff i suppose

That is something i wouldn't accept. You are trying to optimize work which is done automatically by a compiler and move more work which is to be done manually by those who use and maintain the library.

That is something i wouldn't accept. You are trying to optimize work which is done automatically by a compiler and move more work which is to be done manually by those who use and maintain the library.

Allright, lets do it here then

What makes ceres special?

It's an external lib
We rarely update it
We have made no modifications to it

It's not different from llvm/clang or any of the other huge libs we have

It takes up nearly 20% of the total blender build time.

I know it's in /extern since it's a lib that is hard to come by on linux, I'm not suggesting removing it from there, however I also see no reason to make every dev and every full build pay a nearly 20% increase in build time just because this lib is hard to find.

What I was suggesting doing was adding a WITH_SYSTEM_CERES option and precompiling it for the windows users (perhaps mac could benefit here as well).

It is true it wasn't updated in the past year, but mainly because of the amount of bug tracker work.
Every time we update it is either pre-release hash and it is always an API breaking changes (this is an entire point of update -- have new features which are unlocking development).

I do not want to go into a hassle of either doing compile-breaking changes, or pre-compiling library myself, or being pulled down by the latency of platform maintainers catching up with libraries update request. I also do not want to go into hassle with version ifdefs to counteract to that.

Better and more acceptable solution is to disable WITH_LIBMV_SCHUR_SPECIALIZATIONS from blender_full.cmake, and have it only enabled for final releases. having those disabled during development is a good idea anyway, no matter if you're working directly on Libmv/Ceres, or if you don't even plan to touch those.

In D2606#114524, @Ray Molenkamp (LazyDodo) wrote:

I don't think you can have multiple precompiled headers per project so doing this globally would probably be troublesome.

Other C++ code in Blender like Cycles and depsgraph also uses those STL headers. So I'm wondering if just doing these globally would give a bigger speedup overall without much code complexity. Not sure if that might fail to due to different compiler flags though.

Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).Jun 5 2019, 7:00 PM

Cycles won't benefit from precompiled headers all that much, the heaviest items in cycles (kernels) spend most of the build time the optimizer stages due to the crazy amount of inlining we do, making the front end (parsing) cost almost neglectable in comparison.

There may be some savings to be had, but they won't be anywhere near the savings we have here for freestyle, think 0-5% speedup rather than the 3-6x you see with freestyle.

And even then I'd rather finetune it for the specific projects rather than doing it globally, PCH is a precision tool, not something you can/should just slap on project-wide.

In D2606#114532, @Sergey Sharybin (sergey) wrote:

Better and more acceptable solution is to disable WITH_LIBMV_SCHUR_SPECIALIZATIONS from blender_full.cmake, and have it only enabled for final releases. having those disabled during development is a good idea anyway, no matter if you're working directly on Libmv/Ceres, or if you don't even plan to touch those.

That brings it down to about 11% of the total build time, it's without a doubt an improvement, and if I can't have it in svn I'll happily compromise on this option.

That brings it down to about 11% of the total build time, it's without a doubt an improvement, and if I can't have it in svn I'll happily compromise on this option.

Well, good we are moving towards compromise :) Still feels weird. I think i've got different percentage here, but i'm also on Linux.

Wack glog into a PCH? :D Could also help Libmv and, possibly, some areas of Cycles? Eigen could also slow down compilation.

Overall I'm fine with this. Would be nice if we could benefit from it more globally than Freestyle, I would guess it's somehow possible. But I don't mind if this goes in as is.

For Freestyle, we could probably also merge some files together. Like all the files in the Iterator, UnaryFunction0D, .. folders, they wouldn't be that big even after merging. Could also help build times on other platforms.

source/blender/freestyle/FRS_precomp.h
2	python.h -> Python.h

This revision is now accepted and ready to land.Jun 5 2019, 8:29 PM

I setup the blender_precompile_headers macro generic enough that other projects can use it if needed, but it's better to look at those in a case by case basis.

I only took a quick look at gcc their PCH solution seems similar you could probably expand the macro so linux benefits as well but a linux platform dev will have to take a look there.

In D2606#114548, @Sergey Sharybin (sergey) wrote:

That brings it down to about 11% of the total build time, it's without a doubt an improvement, and if I can't have it in svn I'll happily compromise on this option.

Well, good we are moving towards compromise :) Still feels weird. I think i've got different percentage here, but i'm also on Linux.

Wack glog into a PCH? :D Could also help Libmv and, possibly, some areas of Cycles? Eigen could also slow down compilation.

2 year ago when I made this diff for freestyle, I actually also looked at ceres as a candidate for PCH, the results were rather inconclusive and disappointing so I didn't pursue it further

2 year ago when I made this diff for freestyle, I actually also looked at ceres as a candidate for PCH, the results were rather inconclusive and disappointing so I didn't pursue it further

Indeed inconclusive =\

That being said, don't see any issues having this in. The rest Brecht covered quite well.

source/blender/freestyle/FRS_precomp.cpp
2	`FRS_precom.h`, to match case?

In D2606#114565, @Sergey Sharybin (sergey) wrote:

2 year ago when I made this diff for freestyle, I actually also looked at ceres as a candidate for PCH, the results were rather inconclusive and disappointing so I didn't pursue it further

Indeed inconclusive =\

Yeah but given the release and debug build times are so far apart, it doesn't seem likely a whole lot of time is being spent in the front end of the compiler where PCH's would help.

Brecht got me curious about cycles preprocessing, I ran the build twice, once with Preprocess to file and once regular just to get an idea how much time is spend in the preprocessor, there was no option for not writing out the result so the preprocessor segment has some extra cost for IO and the actual savings would be lower than what is shown here.