refactor: StringKey dedup interner + host_test_stubs! macro (036 W5+W6)#143
Open
lxsaah wants to merge 4 commits into
Open
refactor: StringKey dedup interner + host_test_stubs! macro (036 W5+W6)#143lxsaah wants to merge 4 commits into
lxsaah wants to merge 4 commits into
Conversation
…036 W5) intern() leaked a fresh allocation on every call, so re-interning the same key leaked again. A global dedup table (std Mutex / spin Mutex, same pattern as the TypedRecord field locks) now returns the existing 'static allocation for a known key, bounding the leak by the number of *distinct* dynamic keys — the actual lifetime contract of a record key. The contract is documented loudly on intern(), and the debug-build tripwire now counts distinct keys instead of calls. The &'static str / Copy design stays; an Arc<str> key variant remains rejected (forks RecordKey into two shapes — the 034 §3.1 mistake). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion (036 W6) The 18-line no-op #[defmt::global_logger] + panic handler + host time driver block existed in three copies (embassy-adapter session_smoke, embassy-adapter buffer.rs tests, serial-connector embassy_smoke). Per 035 §2.4, an exported host_test_stubs! macro in aimdb-embassy-adapter now holds the single definition; each test binary expands it once, preserving the once-per-binary requirement of #[global_logger]/time_driver_impl!. The time driver uses the wake_by_ref variant (buffer.rs's superset behavior: zero-duration sleeps complete; the smokes never sleep). The serial-connector defmt/embassy-time-driver dev-deps stay — the expansion references them at the invocation site; the Cargo.toml comment now says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, W7 skipped - W3: Nucleo firmware build verified (thumbv8m.main-none-eabihf); single run on a main build is the bar now that #140 is merged. Bench session is the remaining work and the gate for closing 035. - W4: deferred pending W3 scenario-1 AckTimeout evidence (decision 2026-06-12). Design pre-decided for the trigger: buffer the sent frame (option a) — GroupWrite already carries a 254-byte APDU buffer, so the semantic-content variant saves ~350 B total, no real RAM argument. - W7: skipped entirely (decision 2026-06-12); no audit will be run. 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
1 participant
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.
Summary
Implements the two opportunistic S-sized items W5 and W6 from 036-followup-refactoring.md, folded into the W1/W2 series as the doc suggests (stacked on #142 → #141; retarget as the stack merges).
W5 —
StringKey::interndedup interner + loud contract (034 §3.10)intern()leaked a fresh'staticallocation on every call. A global dedup table (std::sync::Mutex/spin::Mutexbehind the same alias-and-lock()pattern as theTypedRecordfield locks) now returns the existing allocation for a known key, so the leak is bounded by the number of distinct dynamic keys — the actual lifetime contract of a record key in a long-lived process. The contract is documented onintern()("each distinct dynamic key allocates once for process lifetime; do not derive keys from unbounded input"), and the debug tripwire (cap 1000) now counts distinct keys instead of calls. The&'static str/Copydesign stays; the lock is held across lookup+leak+insert so a race cannot leak twice. New test asserts pointer-identity for re-interned keys.W6 —
host_test_stubs!macro for the defmt logger triplication (035 §2.4)The 18-line no-op
#[defmt::global_logger]+ panic handler + host time-driver block existed in three copies. A#[macro_export] #[doc(hidden)] macro_rules! host_test_stubsinaimdb-embassy-adapternow holds the single definition; each test binary expands it once (per-binary uniqueness preserved because the expansion is per-binary). The time driver adopts buffer.rs'swake_by_refvariant — the superset behavior (zero-duration sleeps complete; the smokes never sleep). The serial-connectordefmt/embassy-time-driverdev-deps stay: the expansion references them at the invocation site (the 035 "if nothing else needs it" condition doesn't hold); the Cargo.toml comment now explains this.Test plan
make build/make test— full feature matrix, 105 suites green ✅make clippy(-D warnings) +make fmt-check✅cargo test -p aimdb-embassy-adapter --no-default-features --features "alloc,embassy-sync,embassy-time,connectors"andcargo test -p aimdb-serial-connector --no-default-features --features "embassy-runtime"✅test_intern_dedup_returns_same_allocationpasses ✅🤖 Generated with Claude Code