Separate GC WKS and SVR compilation units#126720
Separate GC WKS and SVR compilation units#126720janvorli wants to merge 4 commits intodotnet:mainfrom
Conversation
Move the GC sources away from the wrapper-file model that text-included gc.cpp and gcee.cpp under SERVER_GC and instead compile the shared sources directly as separate WKS and SVR objects. This change introduces gcinternal.h as the shared compilation context for the gc.cpp split, converts the former tail-included GC implementation fragments into separately compiled translation units, and updates the GC, VM, NativeAOT, and GC sample build surfaces to consume the new object layout. It also removes the gcsvr.cpp/gcwks.cpp and gceesvr.cpp/gceewks.cpp wrappers, compiles gcee.cpp through the same dual-build WKS/SVR source lists as gc.cpp, deduplicates the repeated WKS/SVR source lists in the relevant CMake files, and renames the shared GC header from gc_common.h to gcinternal.h to avoid confusion with gccommon.cpp. During the split, cross-translation-unit declarations and inline helpers needed by multiple GC source files were moved into the shared header, while local-only inline helpers were moved back into their owning .cpp files to avoid keeping unnecessary bodies in the shared header. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @agocke, @dotnet/gc |
There was a problem hiding this comment.
Pull request overview
This PR refactors CoreCLR GC build plumbing to stop using wrapper translation units (gcwks.cpp/gcsvr.cpp and gceewks.cpp/gceesvr.cpp) and instead compile the shared GC implementation sources directly into separate WKS/SVR object sets, using a new shared compilation-context header gcinternal.h.
Changes:
- Introduces
gcinternal.hand updates many GC.cppfiles to include it and wrap code inWKS/SVRnamespaces based onSERVER_GC. - Updates CoreCLR VM and standalone GC CMake build graphs to build GC sources as object libraries for WKS/SVR and consume them from coreclr/clrgc targets.
- Updates NativeAOT runtime and the GC sample project build surfaces to consume the new GC source layout.
Reviewed changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/wks/CMakeLists.txt | Adds GC object files into cee_wks_core build inputs. |
| src/coreclr/vm/CMakeLists.txt | Defines shared GC source list and builds vm_gc_wks/vm_gc_svr object libraries. |
| src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt | Links vm_gc_wks/vm_gc_svr into coreclr and coreclr_static. |
| src/coreclr/nativeaot/Runtime/CMakeLists.txt | Replaces wrapper sources with direct GC .cpp compilation for NativeAOT. |
| src/coreclr/gc/gcinternal.h | New shared GC compilation-context header; centralizes includes and inlines. |
| src/coreclr/gc/*.cpp | Switches individual GC implementation files to include gcinternal.h and wrap in WKS/SVR namespaces. |
| src/coreclr/gc/CMakeLists.txt | Builds standalone GC (clrgc/clrgcexp) with WKS/SVR object libraries. |
| src/coreclr/gc/sample/* | Updates GC sample to compile split GC .cpp files directly. |
|
what is the motivation for this change? Does it improve build times? |
It doesn't affect build time in any way. The main reason is code editing experience. The individual files into which the gc.cpp was split in my recent change didn't include the headers for symbols they use, so when editing one of those e.g. in VS code, it was showing a lot of red squiggles and code navigation didn't work well. |
|
Build breaks.... |
| set(SERVER_GC_SOURCES | ||
| ${GC_DIR}/gceesvr.cpp | ||
| ${GC_DIR}/gcsvr.cpp | ||
| ${GC_WKS_SVR_SOURCES} | ||
| ) |
There was a problem hiding this comment.
GC_WKS_SVR_SOURCES is added to COMMON_RUNTIME_SOURCES (line 65) and then also assigned to SERVER_GC_SOURCES (lines 88-90). Since Runtime.ServerGC already includes COMMON_RUNTIME_SOURCES, this causes the same GC .cpp files to be listed twice for the ServerGC target, which can lead to duplicate compilation / object name collisions depending on generator. Consider making SERVER_GC_SOURCES empty (or only truly server-only sources), and rely on target_compile_definitions(Runtime.ServerGC ... SERVER_GC) to build the shared list with the server flavor.
There was a problem hiding this comment.
This is intentional. All the sources GC_WKS_SVR_SOURCES need to be compiled twice - once with SERVER_GC set and once without it. That way we produce workstation GC that's in the common part and server GC that's in the Runtime.ServerGC.
Move the GC sources away from the wrapper-file model that text-included gc.cpp and gcee.cpp under
SERVER_GCand instead compile the shared sources directly as separate WKS and SVR objects.This change introduces gcinternal.h as the shared compilation context for the gc.cpp split, converts the former tail-included GC implementation fragments into separately compiled translation units, and updates the GC, VM, NativeAOT, and GC sample build surfaces to consume the new object layout.
It also removes the gcsvr.cpp/gcwks.cpp and gceesvr.cpp/gceewks.cpp wrappers, compiles gcee.cpp through the same dual-build WKS/SVR source lists as gc.cpp, deduplicates the repeated WKS/SVR source lists in the relevant CMake files, and renames the shared GC header from gc_common.h to gcinternal.h to avoid confusion with gccommon.cpp.
During the split, cross-translation-unit declarations and inline helpers needed by multiple GC source files were moved into the shared header, while local-only inline helpers were moved back into their owning .cpp files to avoid keeping unnecessary bodies in the shared header.
I've made size comparison between the new clrgc.dll, clrgcexp.dll and coreclr.dll and the changes in the gc dlls were very minor, around ~1.5kB growth due to little different decisions of the linker / compiler w.r.t. cold / hot code. The coreclr even became ~1.5kB smaller.