DynamicMessage::for_each_set resolves each field twice and forces nested-message callers to look up again
- Linguagem predominante
- Rust
- Estrelas
- 883
- Forks
- 88
- Merge médio
- 3d 19h
- PRs com merge (30d)
- 42
Descrição
`DynamicMessage`'s `ReflectMessage::for_each_set` does more work per stored field than a visitor over an already-materialised `BTreeMap` needs to, and the shape of its callback forces callers that walk nested messages to look fields up a second time.
Per stored field the loop in `buffa-descriptor/src/reflect/dynamic.rs` does: the `BTreeMap` iteration step; `field_or_extension(number)` (a lookup by number into the message descriptor, falling back to the extension index); `self.has(fd)`, which resolves the field by number *again* to apply the implicit-presence rule to a value the loop already holds; then `value.as_ref()` and a call through `&mut dyn FnMut`. In a profile of a consumer that weighs every set field of a message tree this came to roughly 200 instructions per stored field, most of it in the two lookups.
Two things would help, either alone:
1. Apply the presence rule to the `value` in hand instead of calling `has(fd)` — the implicit-presence check only needs the field's kind and the value, both already available — so each field is resolved once.
2. Offer an inherent, non-dyn road beside the trait method, e.g. `DynamicMessage::set_fields(&self) -> impl Iterator` with the same presence filter and the same extension behaviour. A borrowing iterator also fixes the second problem: the trait callback's `ValueRef<'_>` is not tied to `&self`, and a nested message arrives as `&dyn ReflectMessage`, so a caller that wants to push sub-messages onto a worklist (an iterative walk instead of recursion) cannot keep what the callback hands it and has to call `get`/`field_by_number` again per message field.
The trait method's behaviour would not change; this is a request for a cheaper path for `DynamicMessage` specifically.
Guia de contribuição
Avaliação
Esta issue ainda não foi avaliada.