Repository navigation
⚡ Add mp_units.si, a module with only SI, ISQ and angular - #5
Merged
Merged
Conversation
mp_units.systems includes all 16 unit systems. Most code only uses SI, yet every consumer pays for the whole module: with C++ modules, CMake rebuilds each imported module's interface in every consumer build, and mp_units.systems is the slowest one. Compiling it with LLVM 23 takes 12.3 s and makes a 161 MB BMI; SI, ISQ and angular alone take 3.7 s. mp_units.si holds SI, the ISQ and angular units, and mp_units.systems now imports and re-exports it instead of including those headers itself. Code that only needs SI links mp-units::si and imports mp_units.si. The two modules can be imported together, because each declaration is attached to only one of them. Headers of the other systems include SI headers. They now skip those includes when MP_UNITS_SI_MODULE_IMPORTED is defined, which mp_units.systems does after importing mp_units.si, the same way the headers already skip framework.h inside a module interface. Header-only builds are unchanged. The SI headers move from the systems target to the new si target, and the Conan recipe gets an "si" component.
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
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.
Adds
mp_units.si, a module with only SI, the ISQ and angular units.mp_units.systemsnow imports and re-exports it, so all of mp-units stays available, and code that only needs SI can import much less.Why
With C++ modules, CMake rebuilds the interface of every imported module in each consumer's build.
mp_units.systemsincludes all 16 unit systems and is the slowest of these rebuilds. In libhal's Cortex-M deploys it's rebuilt for every architecture × build type, twice each counting the test_package (libhal/ci#132).Compiling the module interface with LLVM 23, using the same command CMake generates:
mp_units.systemstoday)mp_units.si)Changes
New module:
src/systems/mp-units-si.cppdefinesmp_units.si(angular, isq, isq_angle, si), with CMake targetmp-units::si.mp_units.systems: doesexport import mp_units.si;and definesMP_UNITS_SI_MODULE_IMPORTED, instead of including the SI, ISQ and angular headers itself. The two modules can be imported together, because each declaration is attached to only one of them.Simply including the SI headers in both modules would break that: Clang rejects it with "declaration 'x' attached to named module 'b' cannot be attached to other modules". I confirmed that with a minimal two-module test.
Header guards: 18 headers of the other systems (cgs, iau, codata, hep, iec, natural, typographic, yard_pound, astronomy) skip their SI, ISQ and angular includes when
MP_UNITS_SI_MODULE_IMPORTEDis defined. That follows the existing pattern of skippingframework.hwhenMP_UNITS_IN_MODULE_INTERFACEis defined. Header-only builds are unchanged.Targets: the 26 SI-set headers move from the
systemstarget to the newsitarget, whichsystemslinks. The Conan recipe gets ansicomponent.Testing
conan createwith libhal's options (freestanding,cxx_modules, nostd_format, noimport_std, contracts none, no RTTI) and LLVM 23 passes, including the test_package. The test_package imports the fullmp_unitsumbrella, which brings inmp_units.systemsandmp_units.sitogether.Follow-up
For libhal to benefit, it would import
mp_units.coreplusmp_units.siand linkmp-units::core mp-units::si, instead ofmp_unitsandmp-units::mp-units. That's a separate libhal change.