Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Commits

rB55a2682348df: Windows: Add sccache support.

Summary

sccache [1] is one of the few ccache like solutions that will
work on windows.

Most changes are minor, biggest one is the debug symbol format
ZI stores the symbols in a central .pdb file which makes caching
impossible, Z7 stores it in the actual object files. The only big
thing this breaks is edit & continue but given that's not supported
for ninja anyhow, that is a non issue.

Second change is sccache seeing the experimental options as
something it can't cache, known issue and fix available just
not in any released version yet. [2]

Full build no cache 1428.90s (100.00%)
Full build cached    434.34s ( 30.40%)

I was hoping for better performance, still if I can shave off
17 minutes of every GSOC GA build this summer on a nightly basis
I'd be pretty pleased. The build bots may benefit from this as
well, but we'd have to switch the builds from msbuild to ninja

I consider usage of sccache and advanced use and have purposely
not added it to the help.

[1] https://github.com/mozilla/sccache
[2] https://github.com/mozilla/sccache/pull/724

Diff Detail

Repository

rB Blender

Branch

tmp_sscache (branched from master)

Build Status

Buildable 7625
Build 7625: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.Apr 18 2020, 3:34 AM

Ray Molenkamp (LazyDodo) created this revision.

Harbormaster completed remote builds in B7624: Diff 23867.Apr 18 2020, 3:34 AM

debug may need some more tweaks

Fix double debug symbol flags causing debug caching to misbehave

Harbormaster completed remote builds in B7625: Diff 23868.Apr 18 2020, 6:56 AM

Nicholas Rishel (nicholas_rishel) added a subscriber: Nicholas Rishel (nicholas_rishel).Apr 19 2020, 3:05 AM

Nicholas Rishel (nicholas_rishel) added inline comments.

build_files/cmake/platform/platform_win32.cmake
154–158	Edit: ~~/Z7 may still have some benefits for caching across different folders that /ZI doesn't work for, still investigating.~~ Visual Studio didn't swap /Zi for /ZI leading to cache misses, possibly not running cmake's configure step? I think the SYMBOL_FORMAT related changes can be removed, but leave the change to remove_cc_flag. From sccache source: // -Fd is not taken into account unless -Zi is given // Clang is currently unable to generate PDB files if debug_info && !is_clang { match pdb { Some(p) => outputs.insert("pdb", p), None => { // -Zi without -Fd defaults to vcxxx.pdb (where xxx depends on the // MSVC version), and that's used for all compilations with the same // working directory. We can't cache such a pdb. cannot_cache!("shared pdb"); } }; } https://github.com/mozilla/sccache/blob/211bb2361d2dcbcfc899c89b4a995b579c3be0f6/src/compiler/msvc.rs#L411-L420 /ZI isn't special cased, and it cached correctly when I tested it. The important bit seems to be removing the /Zi above. When I attempted removing /ZI from CMAKE_C[XX]_FLAGS_DEBUG and not removing /Zi I had errors which seem to be described in this sccache issue.
build_files/windows/configure_ninja.cmd
10	Do I understand correctly that this script is only run by make.bat? I had expected changing WITH_WINDOWS_SCCACHE in cmake-gui would append CMAKE_C[XX]_COMPILER_LAUCHER with sccache but I don't think that's the case.

Evan Wilson (EAW) awarded a token.Apr 19 2020, 3:23 AM

Evan Wilson (EAW) added a subscriber: Evan Wilson (EAW).

Visual Studio didn't swap /Zi for /ZI leading to cache misses, possibly not running cmake's configure step?

I don't know what this means, support is currently ninja only so the IDE should not be involved at all?

/ZI isn't special cased, and it cached correctly when I tested

I'm somewhat suspicious here, the issue linked says they can't cache when .pdb's are involved, ZI and Zi both are stored in .pdb files, the only format that logically should work is Z7 as far as i'm able to tell

build_files/windows/configure_ninja.cmd
10	Correct, i'll move this to cmake.

In D7466#179262, @Ray Molenkamp (LazyDodo) wrote:

Visual Studio didn't swap /Zi for /ZI leading to cache misses, possibly not running cmake's configure step?

I don't know what this means, support is currently ninja only so the IDE should not be involved at all?

/ZI isn't special cased, and it cached correctly when I tested

I'm somewhat suspicious here, the issue linked says they can't cache when .pdb's are involved, ZI and Zi both are stored in .pdb files, the only format that logically should work is Z7 as far as i'm able to tell

I read up a little more and I think you're right. The problem seems to be the reuse of the same pdb by multiple object files which is less likely to be the case if specifying the database with the /Fd option. I saw the same behavior when using the /ZI option.

Nicholas Rishel (nicholas_rishel) added inline comments.Apr 20 2020, 3:09 AM

build_files/cmake/platform/platform_win32.cmake
160–161	I lost the link but somewhere mentioned using /DEBUG:FASTLINK to offset some of the link time cost of /Z7. When I timed the with and without it shaved about 2 minutes off build times with sccache, with equivalent cache hits.