1500295 - sccache requests_not_cacheable metric hides some things away

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Mike Hommey [:glandium]]

This alert https://treeherder.mozilla.org/perf.html#/alerts?id=16907 has "improvements" like:

sccache requests_not_cacheable windows2012-32 debug taskcluster-c4.4xlarge 	3,691 	> 	156 	95.77%

following bug 1486554, which was scary at first, because sccache is not supposed to be caching anything on windows builds yet, and it would seem crazy that enabling the clang plugin would start making it cache things. What actually happened, however, is that a whole lot of compilation switched from CannotCache to UnsupportedCompiler as a result of bug 1486554, and those don't appear in any metric.

We should either have an aggregate metric for requests_not_cacheable + requests_unsupported_compiler, or have each tracked individually.

Comment 1

[(not currently active) Ted Mielczarek]

I actually changed this in sccache master so that unsupported compiler makes the client exit with an error:
https://github.com/mozilla/sccache/commit/caf08a7fd2d4449d7c6299c21c991721be38c8e4

This feels like the right choice--sccache should complain loudly if it's being used in a way that will not work.

Comment 2

[Mike Hommey [:glandium]]

Except in that case, the only thing that is happening is that we're enabling the plugin. How does that end up on "unsupported compiler"?

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Mike Hommey [:glandium] from comment #2)
> Except in that case, the only thing that is happening is that we're enabling
> the plugin. How does that end up on "unsupported compiler"?

I'm not sure, TBH. I would have to look at the logs before and after.

Comment 4

[(not currently active) Ted Mielczarek]

Oh, looking at a build log makes it clear. We're now invoking clang.exe and not clang-cl.exe:
03:10:44     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/clang/bin/clang.exe --driver-mode=cl -Xclang -std=gnu99 -fms-compatibility-version=19.15.26726 -m32 -Folz4.obj -c  -DNDEBUG=1 -DTRIMMED=1 -DIMPL_MFBT -DLZ4LIB_VISIBILITY= -Iz:/build/build/src/mfbt -Iz:/build/build/src/obj-firefox/mfbt -Iz:/build/build/src/mfbt/double-conversion -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4244 -wd4267 -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-invalid-noreturn -we4553 -Z7 -Xclang -load -Xclang z:/build/build/src/obj-firefox/build/clang-plugin/clang-plugin.dll -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -WX -Qunused-arguments  -Xclang -MP -Xclang -dependency-file -Xclang .deps/lz4.obj.pp -Xclang -MT -Xclang lz4.obj    z:/build/build/src/mfbt/lz4.c

vs. the same compilation from a log before your change:
23:26:29     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/clang/bin/clang-cl.exe -Xclang -std=gnu99 -fms-compatibility-version=19.15.26726 -m32 -Folz4.obj -c  -DNDEBUG=1 -DTRIMMED=1 -DIMPL_MFBT -DLZ4LIB_VISIBILITY= -Iz:/build/build/src/mfbt -Iz:/build/build/src/obj-firefox/mfbt -Iz:/build/build/src/mfbt/double-conversion -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4244 -wd4267 -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-invalid-noreturn -we4553 -Z7 -O2 -Oy- -WX -Qunused-arguments  -Xclang -MP -Xclang -dependency-file -Xclang .deps/lz4.obj.pp -Xclang -MT -Xclang lz4.obj    z:/build/build/src/mfbt/lz4.c

Comment 5

[(not currently active) Ted Mielczarek]

I hope we can change that back, because supporting `clang.exe --driver-mode=cl` sounds like a nightmare.

Comment 6

[(not currently active) Ted Mielczarek]

This is going to interact with bug 1476604.

Comment 7

[(Away)]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #4)
> Oh, looking at a build log makes it clear. We're now invoking clang.exe and
> not clang-cl.exe:

Shoot, that looks like an unintended effect of ENABLE_CLANG_PLUGIN now happening on the main builds.  https://searchfox.org/mozilla-central/rev/a7f4d3ba4fbfe3efbde832869f1d672fce7122f6/config/static-checking-config.mk#8

Comment 8

[(not currently active) Ted Mielczarek]

While mshal was fixing bug 1476604 we realized that using clang.exe wasn't as big of a deal as I thought. It still reports the same preprocessor defines regardless of whether you run it as clang.exe or clang-cl.exe, so sccache handles it OK. However, I had to land an sccache fix to handle clang.exe:
https://github.com/mozilla/sccache/pull/282

...because sccache was trying to run `clang.exe -showIncludes` to detect the prefix for `-showIncludes` output, and clang does not accept MSVC-style commandline arguments unless you either run it as clang-cl.exe or pass it `--driver-mode=cl`. This means that prior to updating sccache in bug 1476604 we were hitting the "unsupported compiler" code path anyway, because the showIncludes detection was failing.

I did file an sccache issue about more thoroughly handling clang.exe, since anyone that tries to use it without `--driver-mode=cl` is going to have a bad time:
https://github.com/mozilla/sccache/issues/283

...but I think we've fixed all the issues we were hitting in Firefox CI here.