GH-50388: [C++][CI] Make ccache effective MSVC-based builds

[pitrou]

Rationale for this change

By default, C++ debug builds with MSVC output debug information in separate PDB files. However, that is not supported by ccache, hence compilation caching does not work on those builds and they are always slower than desired.

What changes are included in this PR?

On MSVC-based CI builds, embed debug information in object files instead of PDB files, to make ccache effective.

Are these changes tested?

Yes, on existing CI builds.

Sample build times without this PR:

• AMD64 Windows 2022 AVX2 C++: 24m33s compilation (31m37s total)
• AMD64 Windows MSVC GLib: 12m47s compilation (15m29s total)

With this PR:

• AMD64 Windows 2022 AVX2 C++: 3m43s compilation (11m33s total)
• AMD64 Windows MSVC GLib: 3m25s compilation (5m50s total)

Are there any user-facing changes?

No.

• GitHub Issue: [C++][CI] ccache not effective on MSVC builds #50388

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[kou]

+1

Could you remove

arrow/cpp/cmake_modules/SetupCxxFlags.cmake

Lines 238 to 254 in 5bf8934

	if(ARROW_USE_CCACHE)
	foreach(c_flag
	CMAKE_CXX_FLAGS
	CMAKE_CXX_FLAGS_RELEASE
	CMAKE_CXX_FLAGS_DEBUG
	CMAKE_CXX_FLAGS_MINSIZEREL
	CMAKE_CXX_FLAGS_RELWITHDEBINFO
	CMAKE_C_FLAGS
	CMAKE_C_FLAGS_RELEASE
	CMAKE_C_FLAGS_DEBUG
	CMAKE_C_FLAGS_MINSIZEREL
	CMAKE_C_FLAGS_RELWITHDEBINFO)
	# ccache doesn't work with /Zi.
	# See also: https://github.com/ccache/ccache/issues/1040
	string(REPLACE "/Zi" "/Z7" ${c_flag} "${${c_flag}}")
	endforeach()
	endif()

?

[rok]

+1, that is a significant improvement.

[pitrou]

+1

Could you remove

Interesting, I hadn't noticed this piece of CMake code. I assume it wasn't working properly? (perhaps the option was passed as -Zi rather than /Zi?).

Do you think we should set CMAKE_MSVC_DEBUG_INFORMATION_FORMAT in

arrow/cpp/cmake_modules/SetupCxxFlags.cmake

Line 238 in 5bf8934

if(ARROW_USE_CCACHE)

rather than in individual CI config?

[kou]

The CMake code worked properly. I think that it's broken when we use CMake 3.25 or later. I think that CMake 3.25 introduced CMAKE_MSVC_DEBUG_INFORMATION_FORMAT and removed /Zi from CMAKE_C{,XX}_FLAGS*.

So we should use CMAKE_MSVC_DEBUG_INFORMATION_FORMAT instead of the existing /Zi -> /Z7 code.

[pitrou]

I've enabled the C++ Extra tests to check that it also works on ARM64 Windows.

[pitrou]

Ok, cache is now effective on ARM64 Windows as well.