#11622 - PCH is actually NOT enabled for energyplus_tests + fixup PCH usage on Windows CI by jmarrec · Pull Request #11623 · NatLabRockies/EnergyPlus

jmarrec · 2026-06-05T15:05:18Z

Pull request overview

Fixes PCH is actually NOT enabled for energyplus_tests #11622
Functionally revert Improve PCH on MSVC Builds #11529 and replace with a more sane approach

Description of the purpose of this PR

3dfcc1b shows that PCH wasn't added to the energyplus_tests target.

I change the CMake code so that is does, and I added EnergyPlusFixture.hh and gtest/gtest.h to the PCH for energyplus_tests.

before:

time ninja

  real    7m0.262s
  user    54m1.999s
  sys     2m46.176s

Analyzing build trace from 'capture.bin'...
**** Time summary:
Compilation (870 times):
  Parsing (frontend):         2009.9 s
  Codegen & opts (backend):   1639.4 s

after

time ninja

  real    6m34.713s
  user    49m26.037s
  sys     2m27.567s

Analyzing build trace from 'capture_pch.bin'...
**** Time summary:
Compilation (872 times):
  Parsing (frontend):         1659.4 s
  Codegen & opts (backend):   1696.5 s

The expensive headers are gone

**** Expensive headers:
216254 ms: /Users/julien/Software/Others/EnergyPlus/third_party/gtest/googletest/include/gtest/gtest.h (included 282 times, avg 766 ms), included via:
  242x: <direct include>
  22x: AutosizingFixture.hh EnergyPlusFixture.hh
  8x: gtest.h gtest_pred_impl.h
  7x: EnergyPlusFixture.hh
  2x: gmock.h gmock-actions.h gmock-internal-utils.h
  1x: EnergyPlusFixture.hh gtest.h gtest_pred_impl.h

138664 ms: /Users/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh (included 265 times, avg 523 ms), included via:
  233x: <direct include>
  22x: AutosizingFixture.hh
  4x: SQLiteFixture.hh
  3x: CoilCoolingDXFixture.hh SQLiteFixture.hh
  2x: InputProcessorFixture.hh
  1x: ResultsFrameworkFixture.hh
  ...

CI PCH

I started doing very complicated things, but in the end I went with simple:

I don’t think there’s any point restoring a CCACHE cache with a different ENABLE_PCH value, so it’’s part of the base restore key now
run_and_cache_regressions_develop.yml should just do ENABLE_PCH = ON. The OFF case is tested via alternate builds in test_develop_commits.yml. This will reduce the number of shenanigans and confusion.

I originally also added a windows-only hash of the PCH files into the cache keys, but I don't think that's necessary at all, so I reverted it in a9a9da3 but kept a note.

Pull Request Author

Reviewer

…ture and gtest to it ``` **** Expensive headers: 216254 ms: /Users/julien/Software/Others/EnergyPlus/third_party/gtest/googletest/include/gtest/gtest.h (included 282 times, avg 766 ms), included via: 242x: <direct include> 22x: AutosizingFixture.hh EnergyPlusFixture.hh 8x: gtest.h gtest_pred_impl.h 7x: EnergyPlusFixture.hh 2x: gmock.h gmock-actions.h gmock-internal-utils.h 1x: EnergyPlusFixture.hh gtest.h gtest_pred_impl.h 138664 ms: /Users/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh (included 265 times, avg 523 ms), included via: 233x: <direct include> 22x: AutosizingFixture.hh 4x: SQLiteFixture.hh 3x: CoilCoolingDXFixture.hh SQLiteFixture.hh 2x: InputProcessorFixture.hh 1x: ResultsFrameworkFixture.hh ```

…rove_pch_on_msvc" This reverts commit 3be933c, reversing changes made to 690283b.

…mpile_headers at the call site

…H, so stop doing it

… the weirdness. No PCH is tested in test_develop_commits.yml

Functionally revert b032784

…ws if needed

…ame directory (and same as develop run_and_cache workflow) it might breaks MSVC PCH (/FI vs /Yu path mismatch → C1010) Locall on a windows 11 arm64 VM I can build 100% fine with PCH and ccache, using the same versions - ccache: 4.13.6 - ninja: 1.13.2 - MSVC: v17.14.33 (Entreprise on CI, Community on VM) - cmake: 4.3.3

jmarrec · 2026-06-05T15:07:55Z

Since this is touching a workflow that runs only on develop, I pushed a run on my fork https://github.com/jmarrec/EnergyPlus/actions/runs/27041535807

jmarrec · 2026-06-05T15:08:16Z

+  # IMPORTANT: If you change this: modify test_pull_requests.yml in accordance for the ENABLE_PCH value when we found regression tests
+  ENABLE_PCH: ON


Switched this, made a note to keep both workflows in line

jmarrec · 2026-06-05T15:08:27Z

+        # We adjust the ENABLE_PCH to match what is on develop
+        # IMPORTANT: keep this in sync with the ENABLE_PCH from run_and_cache_regressions_develop.yml
+        echo "ENABLE_PCH=ON" >> $GITHUB_ENV


Companion note

jmarrec · 2026-06-05T15:10:45Z

+        key: ccache-${{ matrix.os }}-${{ steps.setup-runner.outputs.compiler-id }}-pch=${{ env.ENABLE_PCH }}-${{ env.CURRENT_SHA }}
        restore-keys: |
-          ccache-${{ matrix.os }}-${{ steps.setup-runner.outputs.compiler-id }}-
-          ccache-${{ matrix.os }}-
+          ccache-${{ matrix.os }}-${{ steps.setup-runner.outputs.compiler-id }}-pch=${{ env.ENABLE_PCH }}-


On develop, the pch is now path of the key. compiler-id too, it won't change nearly as often as we have cache evictions anyways.

key: ccache-ubuntu-24.04-gcc-13-enable-pch=ON-ee1a1b3c25 restore_keys: | ccache-ubuntu-24.04-gcc-13-enable-pch=ON-

key: ccache-windows-2022-msvc-enable-pch=ON-ee1a1b3c25 restore_keys: | ccache-windows-2022-msvc-enable-pch=ON-

jmarrec · 2026-06-05T15:10:55Z

      uses: ./.github/actions/configure-and-build
      with:
-        enable-pch: OFF
+        enable-pch: ${{ env.ENABLE_PCH }}


env var passed here

jmarrec · 2026-06-05T15:14:31Z

    add_executable(${BASE_NAME}_tests ${SRC})
    target_link_libraries(${BASE_NAME}_tests PRIVATE project_options project_warnings)

-    if(USE_PCH)


The issue for #11622 is caused by this.

This is a macro!! In a function, arguments are true variables in the usual CMake sense. In a macro, they are not, they are string replacements much like the C preprocessor would do with a macro

if(USE_PCH)can't work, we MUST use if(${USE_PCH})

https://cmake.org/cmake/help/latest/command/macro.html#argument-caveats

jmarrec · 2026-06-05T15:15:04Z


 # Create test targets
-macro(CREATE_TEST_TARGETS BASE_NAME SRC DEPENDENCIES USE_PCH)
+macro(CREATE_TEST_TARGETS BASE_NAME SRC DEPENDENCIES)


USE_PCH was passed as True/False: it's used by third_party/ObjexxFCL/CMakeLists.txt too, with False on purpose

It's much cleaner to not take that argument, and handle the target_precompile_headers at the call site!

jmarrec · 2026-06-05T15:15:20Z

+create_test_targets(energyplus "${test_src}" "${test_dependencies}")
+if(ENABLE_PCH)
+  target_precompile_headers(energyplus_tests PRIVATE ${EP_TEST_CXX_PCH_HEADERS})
+endif()


Clear AND foolproof

jmarrec · 2026-06-05T15:15:47Z

+set(EP_TEST_CXX_PCH_HEADERS
+    ${EP_CXX_PCH_HEADERS}
+    "$<$<COMPILE_LANGUAGE:CXX>:<gtest/gtest.h>>"
+    "$<$<COMPILE_LANGUAGE:CXX>:${PROJECT_SOURCE_DIR}/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.hh>")


test pch include EnergyPlusFixture.hh and gtest/gtest.h

jmarrec · 2026-06-05T21:51:18Z

      ccache --set-config=max_size=500M
      ccache --set-config=compression=true
      ccache --set-config=sloppiness=pch_defines,time_macros
-      ccache --set-config=base_dir="${{ github.workspace }}"


My guess is that that's the thing that might breaks MSVC PCH (/Yc vs /Yu path mismatch → C1010)

Locally on a windows 11 arm64 VM I can build 100% fine with PCH and ccache, using the same versions

ccache: 4.13.6

ninja: 1.13.2

MSVC: v17.14.33 (Entreprise on CI, Community on VM)

cmake: 4.3.3

jmarrec · 2026-06-05T23:04:58Z

I tested a PR on my fork with two commits: jmarrec#19

First one uses a warm cache from develop, and does a quick non PCH header change
Second one grabs the branch ccache from the previous commit but has a PCH header change

Seems all good. I think base_dir was the culprit indeed.

If this drop we'll see in practice if this still hits PCH issues on windows or not.

jmarrec added 10 commits June 5, 2026 09:24

No PCH on target energyplus_tests

3dfcc1b

Functionally Revert "Merge pull request #11529 from NatLabRockies/imp…

b305425

…rove_pch_on_msvc" This reverts commit 3be933c, reversing changes made to 690283b.

Remove USE_PCH from create_test_targets, clearer to call target_preco…

877800a

…mpile_headers at the call site

On windows, don't pick up a cache with adifferent PCH files

b032784

Minor tweaks

03793b5

There is no point restoring a CCACHE cache with a different ENABLE-PC…

5ee734b

…H, so stop doing it

Just do ENABLE_PCH in run_and_cache_regressions_develop and stop with…

e4ce8e1

… the weirdness. No PCH is tested in test_develop_commits.yml

Don't think it's necessary to compute hash of PCH files on Windows

a9a9da3

Functionally revert b032784

Keep a note for how to reintroduce pch_hash in the cache key on windo…

8383183

…ws if needed

jmarrec added Defect Includes code to repair a defect in EnergyPlus Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc) labels Jun 5, 2026

jmarrec commented Jun 5, 2026

View reviewed changes

jmarrec requested a review from mitchute June 5, 2026 23:03

jmarrec self-assigned this Jun 5, 2026

mitchute approved these changes Jun 8, 2026

View reviewed changes

mitchute merged commit ac87bf0 into develop Jun 8, 2026
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#11622 - PCH is actually NOT enabled for energyplus_tests + fixup PCH usage on Windows CI#11623

#11622 - PCH is actually NOT enabled for energyplus_tests + fixup PCH usage on Windows CI#11623
mitchute merged 11 commits into
developfrom
11622_PCH_and_CI

jmarrec commented Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026

Uh oh!

jmarrec Jun 5, 2026 •

edited

Loading

Uh oh!

jmarrec commented Jun 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# IMPORTANT: If you change this: modify test_pull_requests.yml in accordance for the ENABLE_PCH value when we found regression tests
		ENABLE_PCH: ON

Conversation

jmarrec commented Jun 5, 2026

Pull request overview

Description of the purpose of this PR

CI PCH

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Jun 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmarrec Jun 5, 2026 •

edited

Loading