[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

Fri Nov 22 06:05:49 PST 2019

russell.gallop added a comment.

Thanks for this change. I applied this patch (prior to splitting out https://reviews.llvm.org/D70568) and it built and worked okay (I only see one clang-cl in process explorer).

I don't see anything like same performance improvement however. I did my own assessment of build times. I built 2 x stage 1 toolchains using clang 9.0.0 with and without this patch (applied to 1b9ef3bbb595206b0097b7adec2c1b69eae6fab4 <https://reviews.llvm.org/rG1b9ef3bbb595206b0097b7adec2c1b69eae6fab4>). Then I used each of those to build LLVM (no clang or lld) again.

CMake settings:

  # Build stage 1 (same with and without patch)
  cmake -G Ninja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_LINKER="C:/Program Files/LLVM/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang;lld -DLLVM_TARGETS_TO_BUILD=X86 -DCLANG_ENABLE_STATIC_ANALYZER=OFF -DCLANG_ENABLE_ARCMT=OFF ../llvm

  # Build stage 2 without patch
  cmake -G Ninja -DCMAKE_C_COMPILER="<llvm-project.git>/build_stage1_no_patch/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="<llvm-project.git>/build_stage1_no_patch/bin/clang-cl.exe" -DCMAKE_LINKER="<llvm-project.git>/build_stage1_no_patch/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 ../llvm

  # Build stage 2 with patch
  cmake -G Ninja -DCMAKE_C_COMPILER="<llvm-project.git>/build_stage1_patch/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="<llvm-project.git>/build_stage1_patch/bin/clang-cl.exe" -DCMAKE_LINKER="<llvm-project.git>/build_stage1_patch/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 ../llvm

These both built the patched code.

I got a ~1.5% performance improvement, reducing "ninja all" from 571.5s to 562.5s seconds on a 6 core machine (average over three 3 builds on a quiet machine), times below. For reference I am using winver 1803 and have CFG enabled.

Assuming that `time_saved ≈ (process_invocation_overhead * compiler_jobs) / parallel_build_jobs`
or (rearranging) `process_invocation_overhead ≈ (time_saved * parallel_build_jobs) / compiler_jobs`
(I know this is not perfectly true as build jobs will tail off towards the end of a build, but I think is okay for a ballpark estimate.)

I saved 9 seconds, over 1720 compiler jobs (from `ninja -v -n all | grep -c clang-cl`), with 14 parallel build jobs which gives the process invocation overhead of about 73ms which is in the range you mentioned above (30-100ms). Therefore I think that this is in the right ballpark.

I can't see how this would get 20% (from the table) or 30% improvement (from the first graph). In `abba_test.ps1` you include running tests (due to `check-all`) so it is possible that you are saving time on a very large number of clang driver invocations on very small files. This is helpful for LLVM developers but I don't think it's representative of other builds. Alternatively, is the process overhead higher on your machine(s) for some reason (e.g. security software).

To be clear, I'm not against this patch going in, and I can confirm that it is a performance improvement so good to have. I just can't see where the claimed 20-30% saving on build time comes from.

Time data for 3 builds (seconds)
No patch (571.349, 575.353, 567.638)
With Patch (560.870, 563.110, 563.368)

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69825/new/

https://reviews.llvm.org/D69825