anthropics / anthropics/buffa

Arc<DescriptorPool> refcount shares a cache line with the pool's hot vectors; per-node pool clones contend across threads

Offen
#434 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Rust
Sterne
883
Forks
88
Ø Merge
3 T. 19 Std.
Gemergte PRs (30 T.)
42

Beschreibung

Cross-thread scaling issue in `buffa-descriptor`'s dynamic messages: the `Arc` reference count and the pool's hottest read-only data appear to share a cache line, and every `DynamicMessage` in a tree holds its own clone of that `Arc`.

`DynamicMessage` is `{ pool: Arc, msg_idx, fields, unknown }`, and nested message values are `DynamicMessage`s too, so `DynamicMessage::new`, `clone`, `decode` (once per nested message decoded), `default_value_ref` for an unset message field, and `drop` all write the pool's strong count — one write per message node, per request, from whichever thread is handling that request. Meanwhile `DescriptorPool::message(idx)` — called on essentially every reflective field access — reads `self.messages`' pointer and length. `DescriptorPool` is a plain (non-`repr(C)`) struct whose first fields are `files: Vec<_>` and `messages: Vec<_>`; inside `ArcInner` the strong/weak counts occupy the first 16 bytes, so with the layout rustc currently picks the `messages` header lands within the same 64-byte line as the counts. The effect in a multi-threaded benchmark (N threads, one shared pool, each thread decoding and reading its own messages) is that per-message throughput degrades with thread count even though no thread touches another's message: the count writes invalidate the line the descriptor reads need.

Two possible remedies, not exclusive:

- Layout: keep the reference counts off the line the read path uses — e.g. `#[repr(C)]` on `DescriptorPool` with a leading `[u8; 48]`/`CachePadded` spacer or the cold fields (`files`, `by_name`, `symbols`) first and the hot `messages`/`enums`/`extensions` vectors after the first 64 bytes; cheap, no API change, and worth a `static_assertions`-style layout test so a field reorder does not silently undo it.
- Ownership: let a message tree share one pool handle — nested `DynamicMessage`s inside a `Value::Message` could hold the index only and borrow the root's pool, or the pool clone could be deferred until a nested message is detached from its parent (`take`/`into`). Larger change; removes the per-node count traffic entirely rather than moving it to a quieter line.

Happy to supply the benchmark shape if useful: one `DescriptorPool` in an `Arc`, T threads each looping decode → read a few fields → drop over their own byte buffers, T from 1 to 16, time per iteration on the slowest thread.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.