Core: Link modules together rather than compile them into#605
Open
ChrisJChang wants to merge 17 commits into
Open
Core: Link modules together rather than compile them into#605ChrisJChang wants to merge 17 commits into
ChrisJChang wants to merge 17 commits into
Conversation
…l TUs Add a CMake option (default OFF) that moves a migrated Bit's in-core rollcall macro expansion out of the Core's generated module_rollcall.hpp and into a per-Bit registration translation unit linked only into the gambit executable. The in-core macros already self-register all functors via function-local-static singletons (Core(), Models::ModelDB(), logging registries) at static-init time, so relocating the expansion to a module-owned TU changes nothing at runtime, but means edits to the Bit's rollcall header no longer recompile the Core's largest translation unit. - module_harvester.py/harvesting_tools.py: new -r/--link-time-registration option; listed Bits keep being harvested for functor types, diagnostics and standalone pickles, but their rollcall headers are omitted from module_rollcall.hpp. module_rollcall.hpp is now also only rewritten when its content actually changes, so harvester re-runs no longer dirty the Core needlessly. - executables.cmake: compile <Bit>/registration/<Bit>_registration.cpp into the gambit executable for each migrated Bit. Not added to the Bit's object library, which standalones reuse with their own in-core expansions from standalone_module.hpp. - ExampleBit_A: first (and so far only) migrated Bit. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
The module/backend functor type lists and the module-types rollcall were emitted in Python set iteration order, which varies from run to run due to string-hash randomisation. Every harvester run therefore rewrote module_types_rollcall.hpp (included by types_rollcall.hpp, i.e. by every module TU) with a shuffled include order, forcing near-full rebuilds after touching any rollcall header. Sort all generated lists so the files only change when their actual content changes. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
…urements Utils/static_members.hpp defines static data members and is included via the in-core macro headers, so the per-Bit registration TU duplicated them against the main gambit TU. Honour GAMBIT_NO_STATIC_MEMBER_DEFINITIONS so registration TUs can suppress the definitions, and define it in ExampleBit_A_registration.cpp. Record the rebuild-scope measurements and runtime-equivalence checks in doc/link_time_registration.md, with raw touch-test logs in doc/link_time_registration_logs/: touching ExampleBit_A's rollcall header rebuilds ExampleBit_A.cpp + Core/src/gambit.cpp + link (1m50s) on the legacy path, vs ExampleBit_A.cpp + the registration TU + link (24s) with LINK_TIME_REGISTRATION=On; functor registries and dependency resolution are identical in both configurations. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
…n use ColliderBit currently force-enables the HepMC3 and YODA contribs, making a build impossible in environments where their downloads cannot be performed, even though the source tree carries EXCLUDE_HEPMC/EXCLUDE_YODA guards for exactly this situation (and the corresponding FATAL_ERROR messages already referred to a -DWITH_HEPMC knob). An explicit OFF from the user now downgrades the fatal error to a loud warning and disables the dependent functions, CBS and backends, exactly as when ColliderBit is absent. Defaults are unchanged. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
Second (and first non-trivial) Bit behind LINK_TIME_REGISTRATION. The registration TU expands the full ColliderBit rollcall tree, including the generated ColliderBit_models_rollcall.hpp and Py8Collider_typedefs.hpp and the HAVE_PYBIND11/EXCLUDE_HEPMC/EXCLUDE_YODA-guarded sections; all three guards come from the generated cmake_variables.hpp, so this TU sees the same configuration as the module objects and the legacy in-core expansion. Build ordering w.r.t. the collider harvester holds transitively through the ColliderBit object library, as it already does for module_harvest. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
CORE_CLASSLOAD_NEEDED and CORE_BE_MODEL_RULE call set_classload_requirements and set_backend_rule_for_model, declared in Backends/ini_functions.hpp, which the in-core macro header never included: in the legacy path the declarations arrive only because gambit.hpp happens to include backend_rollcall.hpp before module_rollcall.hpp. Link-time registration TUs (first hit: ColliderBit's NEEDS_CLASSES_FROM(Pythia, default)) include nothing beforehand, so include the declarations where they are used. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
ColliderBit touch: 9m22s -> 7m04s (gambit.cpp no longer rebuilds; the remaining cost is ColliderBit's own 23 sources, which include the rollcall header on either path). Cross-Bit isolation: with ColliderBit in the build, an ExampleBit_A rollcall edit goes from 4m57s (gambit.cpp tax) to 31s. Runtime equivalence: 463/135 registered functors and byte-identical functor tables in both configurations; spartan smoke test passes in both; CBS failure in this YODA-less configuration verified identical with the option OFF and ON (config property, unrelated to registration mode). https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
…RATION The backends do not fit the per-Bit cmake pattern: backend_rollcall.hpp is generated by the backend harvester and #included directly by gambit.hpp, so no harvester change is involved. When LINK_TIME_REGISTRATION is ON, gambit.hpp skips the include (via the global compile definition) and a single registration TU (Backends/registration/Backends_registration.cpp, linked only into the gambit executable) expands it instead; standalones keep their own expansion from standalone_module.hpp. This exposed a real static-initialisation order dependency, previously guaranteed by include order within gambit.cpp's TU: NEEDS_CLASSES_FROM registration (set_classload_requirements) queries backendInfo() version maps populated by the backends' LOAD_LIBRARY registration. With modules and backends in separate TUs, the order is unspecified, and the gambit binary aborted pre-main when ColliderBit registered first. Per the prototype design rule, the requirement is now recorded passively when the backend is not yet known, and retried from backend_info::link_versions each time a backend version registers, making registration order-independent by construction. Requirements that remain unfulfilled after static initialisation (backend never registered at all) are reported by the Core via check_deferred_classload_requirements() at functor-activation time, instead of a pre-main terminate. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
backend_rollcall.hpp and backend_types_rollcall.hpp were emitted in Python set iteration order and rewritten unconditionally on every harvest, so any touch of a frontend header shuffled headers included by every backend (and, via backend_types_rollcall.hpp, most module) translation unit, forcing near-full rebuilds. Same fix as for the module harvester: sort the generated include lists and only rewrite the files when their content actually changes. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
Frontend header touch: 4m32s (gambit.cpp + link) -> 0m55s (single Backends registration TU + link). Runtime equivalence verified: 463/135 registered functors, byte-identical module and backend vertex tables between the two configurations, spartan smoke test passing in both, ExampleBit_A standalone building and running with the deferred classloading mechanism in place. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
CosmoBit, DarkBit, DecayBit, ExampleBit_B, FlavBit, NeutrinoBit, ObjectivesBit, PrecisionBit and SpecBit each gain the standard two-include registration translation unit and an entry in the migrated-Bits list. With LINK_TIME_REGISTRATION=On, module_rollcall.hpp now contains no module rollcall headers at all, so gambit.cpp expands only the model and (legacy path) backend rollcalls. All conditional sections in these rollcall trees (HAVE_PYBIND11, EXCLUDE_FLEXIBLESUSY, FS_MODEL_*_IS_BUILT, ...) are governed by global compile definitions or cmake_variables.hpp, so the registration TUs see the same configuration as the legacy in-core expansion did. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
All 11 Bits + Backends now register at link time when the option is ON; module_rollcall.hpp is then empty of module headers and gambit.cpp expands only the model rollcall. Full-build validation (all Bits, BUILD_FS_MODELS=None): 1654 module + 1610 backend functors, byte-identical functor tables between configurations, spartan smoke test passing in both. DarkBit rollcall touch: 33m12s -> 9m47s; ExampleBit_A rollcall touch in the full build: 49s under link-time registration (legacy number to follow when the timing run completes). https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
Clean timed touch test of ExampleBit_A_rollcall.hpp in the full (all-Bits) legacy configuration: 25m41s, dominated by the gambit.cpp recompile that the legacy path pays for every rollcall edit in any Bit; 49s with link-time registration. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
Link-time registration now covers all Bits and the backends, so remove the stale 'prototype' and 'currently covers ExampleBit_A only' wording from the CMake/BUILD_OPTIONS comments and the design note. No functional change. https://claude.ai/code/session_01PhCtGytokgXUcY6sQ7Yk8C
Collaborator
|
Thanks! Switching these includes in core with linking seems like a generally useful thing, for modularity. Three things we discussed in the Core meeting today:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR changes how module code is brought into core.
Previously, gambit.cpp included all rollcall headers (and backend frontend headers), but it did not actually need this in this file. This meant that if you change any of them, the main gambit.cpp translation unit needed to recompile. This is the slowest part of the compilation, so it makes code development annoyingly slow.
Now, there are new TUs created for each module in a registry folder that simply include the rollcall information. Then these are linked into the main executable. The advantage is that if you change a rollcall header in one module, you don't need to do the slowest recompile again.
Models is currently not included in the changes. This might be the same, but I would need to look into it (it is also a reasonably lightweight module we edit less often).
To turn on this feature, compile with the flag: -DLINK_TIME_REGISTRATION=ON. We can choose to make it default if that is preferred.
This is a change to how the main core interaction with modules operates, so it probably needs a careful look from someone other than just me before merging in.