ITADN

Replace Rc<RefCell<Vec<LocalGlobalPair>>> in MetricsPusher

#163Opensandersaares 创建于 2026-05-24
enhancement
S
sandersaarescommented
## Proposal `MetricsPusher` holds its registered pair list as `Rc<RefCell<Vec<LocalGlobalPair>>>` (the exact shape — verify in `packages/nm/src/pusher.rs`). This has three costs: 1. **`Rc` overhead**: every clone of the pusher handle bumps a refcount (non-atomic but still a write + branch). Cloning happens whenever an `Event<Push>` is registered. 2. **`RefCell` overhead**: every `push()` and every registration borrows the inner cell at runtime (counter increment + branch + decrement). On the push hot path this is non-trivial. 3. **`Vec` reallocation churn**: if events are registered after some have been registered, the `Vec` may reallocate, invalidating any stored pointers (the dirty-list issue #05 needs stable indices for this reason). Replace with a structure that: - Uses a `Cell<*const _>` or arena-like storage instead of `RefCell<Vec<_>>`, OR - Uses an intrusive linked list of registered pairs (each pair owns its own list-node, no `Vec`), OR - Stores pairs in a `Vec` inside a `Cell` (not `RefCell`) and uses `Cell::with_mut`-style scoped access (no runtime borrow tracking). For the single-threaded push pusher, the strongest option is **`Cell<Option<Box<LocalGlobalPair>>>` head + intrusive `next` pointer in each `LocalGlobalPair`**. This: - Eliminates `RefCell`'s borrow-counter on every push tick. - Gives stable addresses for each pair (helpful for dirty-list designs). - Allows O(1) registration without reallocation. ## Baseline | Scenario | Instructions | Cycles | |-----------------------------------------|--------------|--------| | Push idle (4 push pairs, 0 dirty) | 185 | 354 | | Push dirty (4 push pairs, 4 dirty) | 263 | 788 | A `RefCell::borrow()` is ~5 instructions; the `Drop` of the `Ref` is another ~5. Per push tick, that's ~10 instructions of pure bookkeeping. ## Expected magnitude - 5–10 instructions per push tick (constant; not per-pair). - More important: enables clean implementation of issue #05 (dirty list) and improves the cache layout of the pair list (each pair is its own allocation, but the head pointer + first pair share locality). ## Implementation strategy ### Phase A — drop `RefCell` for `Cell` If interior mutability is the only thing `RefCell` provides (no need to hand out `&mut` references that survive across method boundaries), replace with `Cell` + `take`/`set` swap, or use unsafe-but-checked single-threaded patterns: ```rust struct PusherInner { // each access takes and replaces the Vec inside the Cell pairs: Cell<Vec<LocalGlobalPair>>, } ``` `Cell::with_mut` (a workspace helper, if one exists) or scoped `take`/`set` patterns avoid the RefCell counter. ### Phase B — intrusive linked list Replace the `Vec<LocalGlobalPair>` with an intrusive linked list: ```rust struct LocalGlobalPair { // ... existing fields ... next: Cell<Option<NonNull<LocalGlobalPair>>>, } struct PusherInner { head: Cell<Option<NonNull<LocalGlobalPair>>>, } ``` Each pair is pinned (registered events are pinned anyway because they live behind `Rc`). Registration prepends to the list; iteration walks `next`. Note: this is `unsafe` territory. Validate with Miri. Existing crate uses similar patterns elsewhere — check if `intrusive_collections` or a workspace abstraction is already used (it is not based on current exploration). ### Phase C — drop `Rc` if possible If the pusher handle is only cloned within a single thread and lifetimes are tractable, replace `Rc<RefCell<Inner>>` with `&Inner` borrows or owned `Inner` types. Investigate whether all `Rc` clones are necessary. ## Validation 1. `wsl -e bash -l -c "just package=nm bench-cg"` push idle / push dirty scenarios. Confirm reduction. 2. `just package=nm miri-harder` — **critical** if Phase B (intrusive list) is adopted. 3. `just package=nm test` for correctness. ## Risk Medium to high if Phase B (intrusive list) is adopted — it introduces `unsafe`. Mitigate with extensive Miri runs. Phase A alone is low risk. ## Related - Should be considered together with issue #05 (dirty-pair list), as both touch the same data structure. Landing them together avoids re-touching the structure twice.
0 条评论