Skip to content

Conversation

@jstarks
Copy link
Contributor

@jstarks jstarks commented Oct 28, 2025

ValueSets contain both a FieldSet reference and a slice of (&Field, Option<&dyn Value>) pairs. In cases where ValueSets are generated via documented interfaces (specifically, tracing::event! and other macros), the Field references are redundant, because the ValueSet contains a value slot for every field (either a value or None), in the correct order.

As a result, the code generated by the macros is terrible--it must put a Field on the stack for each field--that's 32 bytes per field! This is a lot of work for apparently no purpose at runtime, and it can't be moved into a read-only data section since it's intermixed with dynamic data.

Fix this by adding a variant of ValueSet that skips the Field references, knowing that it represents the full set of fields. Keep the old kind of ValueSet, too--it's still needed by Span::record, by old versions of crates, and potentially by third-party crates using undocumented methods such as FieldSet::value_set.

In some adhoc tests on x86_64 Linux, this reduces the code size as follows:

  • One-field event: 258 bytes to 189 bytes, 25% reduction.
  • Five-field event: 638 bytes to 276 bytes, 57% reduction.
  • In a larger project with lots of events, ~5% reduction in .text section.

`ValueSet`s contain both a `FieldSet` reference and a slice of
(`&Field`, `Option<&dyn Value>`) pairs. In cases where `ValueSet`s are
generated via documented interfaces (specifically, `tracing::event!` and
other macros), the `Field` references are redundant, because the
`ValueSet` contains a value slot for every field (either a value or
`None`), in the correct order.

As a result, the code generated by the macros is terrible--it must
put a `Field` on the stack for each field--that's 32 bytes per field!
This is a lot of work for apparently no purpose at runtime, and it
can't be moved into a read-only data section since it's intermixed with
dynamic data.

Fix this by adding a variant of `ValueSet` that skips the `Field`
references, knowing that it represents the full set of fields. Keep
the old kind of `ValueSet`, too--it's still needed by `Span::record`,
by old versions of crates, and potentially by third-party crates using
undocumented methods such as `FieldSet::value_set`.

In some adhoc tests on x86_64 Linux, this reduces the code size as
follows:
* One-field event: 258 bytes to 189 bytes, 25% reduction.
* Five-field event: 638 bytes to 276 bytes, **57%** reduction.
* In a larger project with lots of events, ~5% reduction in .text section.
@jstarks jstarks requested review from a team and hawkw as code owners October 28, 2025 17:01
@jstarks
Copy link
Contributor Author

jstarks commented Oct 28, 2025

Test and clippy failures are pre-existing. But the test failure is a little alarming since it's in the area that I'm changing. Looking into it.

@jstarks
Copy link
Contributor Author

jstarks commented Oct 28, 2025

I see the test bug. Someone regressed the test last year while cleaning up warnings. I will open a separate PR to fix this.

@jstarks
Copy link
Contributor Author

jstarks commented Oct 28, 2025

#3399

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant