Cut build times: PCH tuning and a hook instantiation fix (#1813)

7 files changed, 72 insertions, 8 deletions

diff --git a/CMake/DefaultCXX.cmake b/CMake/DefaultCXX.cmake
index b3042365a..71dd1f9ab 100644
--- a/CMake/DefaultCXX.cmake
+++ b/CMake/DefaultCXX.cmake
@@ -9,7 +9,9 @@ if(MSVC)
     set_target_properties("${PROPS_TARGET}" PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
     set_config_specific_property("DEFAULT_CXX_EXCEPTION_HANDLING" "/EHsc")
 
-    if (CMAKE_C_COMPILER_LAUNCHER MATCHES "ccache|sccache")
+    # /Zi funnels every parallel cl.exe through one mspdbsrv.exe writing a shared PDB, and is not
+    # cacheable by ccache/sccache. /Z7 keeps the debug info in the .obj instead.
+    if (CMAKE_C_COMPILER_LAUNCHER MATCHES "ccache|sccache" OR NOT CMAKE_GENERATOR MATCHES "Visual Studio")
         set_config_specific_property("DEFAULT_CXX_DEBUG_INFORMATION_FORMAT" "/Z7")
     else()
         set_config_specific_property("DEFAULT_CXX_DEBUG_INFORMATION_FORMAT" "/Zi")
diff --git a/CMakeLists.txt b/CMakeLists.txt
index e9280654a..11b37cc73 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -165,6 +165,25 @@ add_compile_definitions(CONTROLLERBUTTONS_T=uint32_t)
 ################################################################################
 add_subdirectory(libultraship ${CMAKE_BINARY_DIR}/libultraship)
 target_compile_definitions(libultraship PUBLIC INCLUDE_MPQ_SUPPORT)
+
+# LUS ships no PCH of its own and these headers dominate its build. Done here rather than in the
+# submodule, and PRIVATE, so nothing about its public interface changes.
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|AppleClang|GNU|MSVC")
+    target_precompile_headers(libultraship PRIVATE
+        "$<$<COMPILE_LANGUAGE:CXX>:<ship/Context.h$<ANGLE-R>>"
+        "$<$<COMPILE_LANGUAGE:CXX>:<ship/resource/ResourceManager.h$<ANGLE-R>>"
+        "$<$<COMPILE_LANGUAGE:CXX>:<ship/window/Window.h$<ANGLE-R>>"
+        "$<$<COMPILE_LANGUAGE:CXX>:<spdlog/spdlog.h$<ANGLE-R>>"
+    )
+    # Clang replays a PCH's pending template instantiations in every consuming TU unless told to do
+    # the work up front. MSVC's PCH already behaves this way.
+    if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|AppleClang")
+        target_compile_options(libultraship PRIVATE
+            $<$<COMPILE_LANGUAGE:CXX>:-fpch-instantiate-templates>
+        )
+    endif()
+endif()
+
 add_subdirectory(ZAPDTR/ZAPD ${CMAKE_BINARY_DIR}/ZAPD)
 add_subdirectory(OTRExporter)
 add_subdirectory(mm)
diff --git a/mm/2s2h/GameInteractor/GameInteractor.cpp b/mm/2s2h/GameInteractor/GameInteractor.cpp
index 5770e84ca..25392e6d5 100644
--- a/mm/2s2h/GameInteractor/GameInteractor.cpp
+++ b/mm/2s2h/GameInteractor/GameInteractor.cpp
@@ -587,6 +587,13 @@ void ProcessEvents(Actor* actor) {
     GameInteractor::Instance->events.erase(GameInteractor::Instance->events.begin());
 }
 
+// On MSVC this is defined inline in the header instead; see the declaration for why.
+#ifndef _MSC_VER
+void GameInteractor::RemoveAllQueuedHooks() {
+#include "GameInteractor_RemoveAllQueuedHooks.inc"
+}
+#endif
+
 void GameInteractor::RegisterOwnHooks() {
     // Cleanup all hooks at the start of each frame
     GameInteractor::Instance->RegisterGameHook<GameInteractor::OnGameStateMainStart>(
diff --git a/mm/2s2h/GameInteractor/GameInteractor.h b/mm/2s2h/GameInteractor/GameInteractor.h
index 58c4fe1f2..fdbed2287 100644
--- a/mm/2s2h/GameInteractor/GameInteractor.h
+++ b/mm/2s2h/GameInteractor/GameInteractor.h
@@ -443,13 +443,18 @@ class GameInteractor {
         HooksToUnregister<H>::hooksForFilter.clear();
     }
 
+    // Inline on MSVC, out of line elsewhere, and that difference is load-bearing. The body
+    // instantiates ProcessUnregisteredHooks<> for every hook in the table. MSVC's PCH is a state
+    // snapshot, so doing it inside the PCH hands the result to every TU for free; Clang's PCH
+    // replays pending instantiations per TU, where the same inline definition dominated even an
+    // empty TU. Measure before changing this.
+#ifdef _MSC_VER
     void RemoveAllQueuedHooks() {
-#define DEFINE_HOOK(name, _) ProcessUnregisteredHooks<name>();
-
-#include "GameInteractor_HookTable.h"
-
-#undef DEFINE_HOOK
+#include "GameInteractor_RemoveAllQueuedHooks.inc"
     }
+#else
+    void RemoveAllQueuedHooks();
+#endif
 
     class HookFilter {
       public:
diff --git a/mm/2s2h/GameInteractor/GameInteractor_RemoveAllQueuedHooks.inc b/mm/2s2h/GameInteractor/GameInteractor_RemoveAllQueuedHooks.inc
new file mode 100644
index 000000000..2a964387a
--- /dev/null
+++ b/mm/2s2h/GameInteractor/GameInteractor_RemoveAllQueuedHooks.inc
@@ -0,0 +1,8 @@
+// Body of GameInteractor::RemoveAllQueuedHooks, which is defined in the header on MSVC and in the
+// .cpp everywhere else. See the declaration in GameInteractor.h for why.
+
+#define DEFINE_HOOK(name, _) ProcessUnregisteredHooks<name>();
+
+#include "GameInteractor_HookTable.h"
+
+#undef DEFINE_HOOK
diff --git a/mm/CMake/DefaultCXX.cmake b/mm/CMake/DefaultCXX.cmake
index b3042365a..71dd1f9ab 100644
--- a/mm/CMake/DefaultCXX.cmake
+++ b/mm/CMake/DefaultCXX.cmake
@@ -9,7 +9,9 @@ if(MSVC)
     set_target_properties("${PROPS_TARGET}" PROPERTIES MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
     set_config_specific_property("DEFAULT_CXX_EXCEPTION_HANDLING" "/EHsc")
 
-    if (CMAKE_C_COMPILER_LAUNCHER MATCHES "ccache|sccache")
+    # /Zi funnels every parallel cl.exe through one mspdbsrv.exe writing a shared PDB, and is not
+    # cacheable by ccache/sccache. /Z7 keeps the debug info in the .obj instead.
+    if (CMAKE_C_COMPILER_LAUNCHER MATCHES "ccache|sccache" OR NOT CMAKE_GENERATOR MATCHES "Visual Studio")
         set_config_specific_property("DEFAULT_CXX_DEBUG_INFORMATION_FORMAT" "/Z7")
     else()
         set_config_specific_property("DEFAULT_CXX_DEBUG_INFORMATION_FORMAT" "/Zi")
diff --git a/mm/CMakeLists.txt b/mm/CMakeLists.txt
index 9f0834a7a..d9f7f09c5 100644
--- a/mm/CMakeLists.txt
+++ b/mm/CMakeLists.txt
@@ -328,7 +328,11 @@ if(MSVC)
         ${DEFAULT_CXX_DEBUG_INFORMATION_FORMAT};
         ${DEFAULT_CXX_EXCEPTION_HANDLING}
     )
-    target_compile_options(${PROJECT_NAME} PRIVATE  $<$<CONFIG:Debug>:/ZI;>)
+    # /ZI is Edit and Continue, usable only by the Visual Studio debugger. It overrides the /Z7 or
+    # /Zi chosen above, and forces every parallel cl.exe through one mspdbsrv.exe.
+    if (CMAKE_GENERATOR MATCHES "Visual Studio")
+        target_compile_options(${PROJECT_NAME} PRIVATE  $<$<CONFIG:Debug>:/ZI;>)
+    endif()
     target_link_options(${PROJECT_NAME} PRIVATE
         $<$<CONFIG:Debug>:
             /INCREMENTAL
@@ -491,6 +495,23 @@ target_precompile_headers(${PROJECT_NAME} PRIVATE
     "$<$<COMPILE_LANGUAGE:C>:${CMAKE_CURRENT_SOURCE_DIR}/include/variables.h>"
 )
 
+# MSVC only: the header is expensive to parse there, few sources include it, and MSVC's PCH load
+# barely grows with content. On Clang the parse is cheap by comparison and the bigger PCH is paid
+# for by every source in the target, making it a net loss.
+if (MSVC)
+    target_precompile_headers(${PROJECT_NAME} PRIVATE
+        "$<$<COMPILE_LANGUAGE:CXX>:${CMAKE_CURRENT_SOURCE_DIR}/2s2h/BenGui/UIWidgets.hpp>"
+    )
+endif()
+
+# Instantiate templates once while building the PCH instead of replaying them in every TU that
+# consumes it. Slightly slower and larger PCH, much cheaper for every object that uses it.
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|AppleClang")
+    target_compile_options(${PROJECT_NAME} PRIVATE
+        $<$<COMPILE_LANGUAGE:CXX>:-fpch-instantiate-templates>
+    )
+endif()
+
 ################################################################################
 # Pre build events
 ################################################################################