-
-
Notifications
You must be signed in to change notification settings - Fork 724
PERF/STYLE: Investigate code bloat from inline destructors vs. out-of-line definitions #6000
Copy link
Copy link
Open
Description
Background
During review of PR #5995 (and related smart-pointer PRs #5994–#5997), N-Dekker raised two
concerns about moving class destructors from .cxx to inline = default in headers:
- Code bloat — an inline destructor is emitted in every translation unit (TU) that
destroys an instance; a non-inline definition is emitted once and shared via the DSO. - ABI break — for
ITKFEM_EXPORT(and otherITK*_EXPORT) classes, moving the
destructor from a non-inline exported symbol to an inline one is a binary-incompatible
change for downstream shared-library consumers.
This issue records a plan to measure the magnitude of the effect and produce data to inform
a policy decision for the ITK codebase.
Research Questions
- What is the measured difference in object-file and shared-library size when destructors
are inline= defaultvs. defined in.cxx? - Does the effect vary by compiler (GCC, Clang, MSVC) or optimization level (
-O0,-O2,-Os)? - Is the effect significant enough to warrant a blanket policy, or only for specific
high-fan-out classes (e.g., image filters instantiated across many TUs)? - What ABI/symbol-visibility change does each approach produce (check with
nm/objdump)?
Proposed Investigation Plan
Step 1 — Enumerate candidate destructors
# Find exported ITK classes that have an out-of-line destructor defined in a .cxx
grep -rn "^[A-Za-z:]*::~[A-Za-z]*()" Modules/*/src/ Modules/*/include/ \
| grep "= default" | grep -v ThirdPartyProduce a representative sample of:
- Leaf classes (no subclasses) — destructor called in many TUs
- Abstract base classes — virtual destructor, called through pointer
- Template-explicit-instantiation classes — e.g.,
itkImage.hxx
Step 2 — Build baseline (all destructors in .cxx)
cmake -DITK_BUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release \
-DCMAKE_CXX_FLAGS="-Os" ..
ninja ITKCommon ITKFEM ITKIOImageBase
# Record: .so sizes, nm --defined-only -S output for each DSO
du -sh lib/libITKCommon*.so lib/libITKFEM*.so lib/libITKIOImageBase*.so
nm --defined-only -S lib/libITKCommon*.so | grep " T " | wc -lStep 3 — Apply patch (move destructors to inline = default)
Write a script that:
- Detects
ClassName::~ClassName() = default;in.cxxfiles - Moves them to
= defaulton the declaration in the.hfile - Removes the
.cxxentry
# Suggested script location
Utilities/Scripts/move_destructors_to_header.pyRebuild with identical flags and record the same metrics.
Step 4 — Measure per-TU object file size
For a high-fan-out class (e.g., itk::DataObject), compare:
# Count how many TUs emit the destructor symbol
nm --defined-only build/CMakeFiles/**/*.o | grep "DataObject::~DataObject" | wc -l
# Compare total size of all .o files before and after
find build/CMakeFiles -name "*.o" | xargs du -c | tail -1Step 5 — Measure link-time and startup overhead
time ninja ITKCommon # full rebuild link time
ldd --verbose lib/libITKCommon*.so | grep "lazy" # optional: startup costStep 6 — Check ABI / symbol visibility
# Before (out-of-line):
nm -D lib/libITKCommon*.so | grep "~DataObject"
# Expected: T (strong global text symbol)
# After (inline = default):
nm -D lib/libITKCommon*.so | grep "~DataObject"
# Expected: W (weak symbol) or absentDocument whether moving to inline changes from T → W and whether this is
an ABI concern for ITK's versioned shared libraries.
Step 7 — Compiler matrix
Repeat Steps 2–6 with:
| Compiler | Version | Flags |
|---|---|---|
| GCC | 12 | -O2 -Os |
| Clang | 16 | -O2 -Os |
| MSVC | 2022 | /O2 /Os |
Step 8 — Summarize and propose policy
Based on measurements, recommend one of:
- A: Keep all destructors out-of-line in
.cxx(minimize bloat, preserve ABI stability) - B: Allow inline
= defaultonly for non-exported helper/internal classes - C: Allow inline
= defaulteverywhere — bloat is negligible at ITK's scale - D: Adopt a per-class decision based on fan-out heuristic
Acceptance Criteria
- Numerical binary-size data for at least 3 ITK modules under GCC and Clang
- Symbol-visibility comparison (
nmoutput) before/after for at least one exported class - Recommendation with justification posted as a comment on this issue
- Update
Documentation/AI/compiler-cautions.md(section 1b) with the adopted policy
References
- PR COMP: Use std::unique_ptr in FEMLinearSystemWrapperDenseVNL #5995 (N-Dekker review comment): COMP: Use std::unique_ptr in FEMLinearSystemWrapperDenseVNL #5995 (comment)
- Scott Meyers, Effective Modern C++, Item 22
- Itanium C++ ABI — Vague Linkage (weak symbols for inline functions)
- GCC documentation on
-fvisibilityand inline destructors
🤖 Generated with Claude Code (claude-sonnet-4-6)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels