Repository navigation
Modernization Phase 2.4: streamr-dht modules + bench checkpoint — pivot to coarse façade partitions [iosbuild] - #32
Merged
Merged
Conversation
…artitions (iosbuild) The checkpoint caught a real architecture flaw by measurement: per-header facade partitions multiplied compile costs (clean 102s -> 238s; SLogger touch 65s -> 336s) because every partition re-parsed the folly/protobuf stack in its GMF and the BMI chain amplified rebuild cascades. Design revised (all packages): ONE :all facade partition per package (+ :protos over generated code). Import interface unchanged. Result: clean build 83s (-19% vs pre-modules baseline) and SLogger touch 59s (baseline parity) already at the facade stage; per-header granularity returns at consolidation where it pays. Also in this phase: - streamr-dht migrated: :protos over the DhtRpc trio (35 messages, 6 enums, 9+9 generated stubs) + :all over 46 headers; 43 test TUs flipped to import (exactly 3 one-line fixes needed). - homebrewClang.cmake: removed the global $HOMEBREW_PREFIX/include standard-include-dirs — exported module targets baked them in and consumer-side synthesized BMI compiles ordered them first, letting a brew-installed folly/fmt silently shadow the vcpkg ones. - Rule hardened: import-using TUs must not textually include sibling headers reaching the same third-party stacks (clangd false ODR); remaining sibling includes flipped to imports, zero new exclusions. - 6 more constants -> inline constexpr. Verified: dht 81/81, utils 49/49 via import; tn + proxyclient downstream unchanged; root 307/307; full lint green; bench numbers recorded in MODERNIZATION.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (iosbuild) CI (--prod/Release) exposed the second instance of the TU-local-in-GMF disease: folly/fibers/FiberManagerInternal-inl.h defines helper function templates in an ANONYMOUS namespace; with the header reached both through module GMFs and textually, clang emits duplicate identical mangled names (FOLLY_NOINLINE forces emission in Release): error: definition with same mangled name '_ZN5folly6fibers12_GLOBAL__N_111runNoInline...' as another definition Overlay patch names the namespace + using-namespace for call sites (external linkage -> ODR-mergeable, definitions identical). Same class as the protobuf VarintParseSlowArm patch; documented in JUSTIFICATION.md. Also: PendingConnectionTest's folly include normalized to the experimental/ shim path used everywhere else. Verified: dht Release build + 81/81; all 8 packages Debug + root 307/307; lint; bench clean 88s (unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…red ubuntu runner (documented pre-existing socket-test workstream) (iosbuild) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
The major bench checkpoint did exactly what it exists for: it caught an architecture flaw by measurement, and the fix turned a 2.3× regression into the migration's first real win.
The checkpoint story
The plan's original façade design — one partition per public header — multiplied compile costs at dht scale: 45 partitions each re-parsed the folly/protobuf header stack in their global module fragments, and the BMI dependency chain amplified incremental cascades (every partition BMI of logger→utils→proto-rpc→dht rebuilt in sequence before any test TU).
SLogger.hppincrementalStreamID.hppincrementalDesign revision (all packages, in this PR): each package exposes ONE
:allfaçade partition (+:protoswhere generated code exists). Theimport streamr.<pkg>;interface is unchanged; per-header granularity was only ever needed for consolidation-stage code placement and returns there. Clean build is already 19 % under the pre-modules baseline at the façade stage — before any consolidation gains — because the ~100 test TUs now load BMIs instead of re-parsing header stacks.streamr-dht migration (the payload)
:protoswraps the DhtRpc trio — 35 messages, 6 enums (using enumfor enumerators), 9+9 generated client/server stubs. The 2.3 protobuf overlay patch held for the 12k-line header.:allover all 46 public headers, 6 nested event namespaces re-exported.import streamr.dht— the flip needed exactly three one-line fixes.Two more durable findings
homebrewClang.cmakeinjected$HOMEBREW_PREFIX/includeas standard include dirs; exported module targets baked them in, and consumer-side synthesized BMI compiles ordered them FIRST — a brew-installed folly/fmt silently shadowed vcpkg's. The global include dirs are gone (keg clang finds its own headers; all deps come from vcpkg). Latent footgun for any machine with brew C++ libraries installed, exposed only by the modules export machinery.Verification
import[iosbuild])Next
Phase 2.5: trackerless-network + proxyclient (final metrics), then the 2.6 consolidations.
🤖 Generated with Claude Code