Skip to content

Commit 4bf3fef

Browse files
authored
tracing: improve code generation at trace points significantly (#3398)
`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.
1 parent c47c777 commit 4bf3fef

File tree

2 files changed

+131
-124
lines changed

2 files changed

+131
-124
lines changed

tracing-core/src/field.rs

Lines changed: 75 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,18 @@ pub struct FieldSet {
165165

166166
/// A set of fields and values for a span.
167167
pub struct ValueSet<'a> {
168-
values: &'a [(&'a Field, Option<&'a (dyn Value + 'a)>)],
168+
values: Values<'a>,
169169
fields: &'a FieldSet,
170170
}
171171

172+
enum Values<'a> {
173+
/// A set of field-value pairs. Fields may be for the wrong field set, some
174+
/// fields may be missing, and fields may be in any order.
175+
Explicit(&'a [(&'a Field, Option<&'a (dyn Value + 'a)>)]),
176+
/// A list of values corresponding exactly to the fields in a `FieldSet`.
177+
All(&'a [Option<&'a (dyn Value + 'a)>]),
178+
}
179+
172180
/// An iterator over a set of fields.
173181
#[derive(Debug)]
174182
pub struct Iter {
@@ -922,7 +930,22 @@ impl FieldSet {
922930
{
923931
ValueSet {
924932
fields: self,
925-
values: values.borrow(),
933+
values: Values::Explicit(values.borrow()),
934+
}
935+
}
936+
937+
/// Returns a new `ValueSet` for `values`. These values must exactly
938+
/// correspond to the fields in this `FieldSet`.
939+
///
940+
/// If `values` does not meet this requirement, the behavior of the
941+
/// constructed `ValueSet` is unspecified (but not undefined). You will
942+
/// probably observe panics or mismatched field/values.
943+
#[doc(hidden)]
944+
pub fn value_set_all<'v>(&'v self, values: &'v [Option<&'v (dyn Value + 'v)>]) -> ValueSet<'v> {
945+
debug_assert_eq!(values.len(), self.len());
946+
ValueSet {
947+
fields: self,
948+
values: Values::All(values),
926949
}
927950
}
928951

@@ -1033,13 +1056,24 @@ impl ValueSet<'_> {
10331056
///
10341057
/// [visitor]: Visit
10351058
pub fn record(&self, visitor: &mut dyn Visit) {
1036-
let my_callsite = self.callsite();
1037-
for (field, value) in self.values {
1038-
if field.callsite() != my_callsite {
1039-
continue;
1059+
match self.values {
1060+
Values::Explicit(values) => {
1061+
let my_callsite = self.callsite();
1062+
for (field, value) in values {
1063+
if field.callsite() != my_callsite {
1064+
continue;
1065+
}
1066+
if let Some(value) = *value {
1067+
value.record(field, visitor);
1068+
}
1069+
}
10401070
}
1041-
if let Some(value) = value {
1042-
value.record(field, visitor);
1071+
Values::All(values) => {
1072+
for (field, value) in self.fields.iter().zip(values.iter()) {
1073+
if let Some(value) = *value {
1074+
value.record(&field, visitor);
1075+
}
1076+
}
10431077
}
10441078
}
10451079
}
@@ -1050,28 +1084,42 @@ impl ValueSet<'_> {
10501084
/// [visitor]: Visit
10511085
/// [`ValueSet::record()`]: ValueSet::record()
10521086
pub fn len(&self) -> usize {
1053-
let my_callsite = self.callsite();
1054-
self.values
1055-
.iter()
1056-
.filter(|(field, _)| field.callsite() == my_callsite)
1057-
.count()
1087+
match self.values {
1088+
Values::Explicit(values) => {
1089+
let my_callsite = self.callsite();
1090+
values
1091+
.iter()
1092+
.filter(|(field, _)| field.callsite() == my_callsite)
1093+
.count()
1094+
}
1095+
Values::All(values) => values.len(),
1096+
}
10581097
}
10591098

10601099
/// Returns `true` if this `ValueSet` contains a value for the given `Field`.
10611100
pub(crate) fn contains(&self, field: &Field) -> bool {
1062-
field.callsite() == self.callsite()
1063-
&& self
1064-
.values
1101+
if field.callsite() != self.callsite() {
1102+
return false;
1103+
}
1104+
match self.values {
1105+
Values::Explicit(values) => values
10651106
.iter()
1066-
.any(|(key, val)| *key == field && val.is_some())
1107+
.any(|(key, val)| *key == field && val.is_some()),
1108+
Values::All(values) => values[field.i].is_some(),
1109+
}
10671110
}
10681111

10691112
/// Returns true if this `ValueSet` contains _no_ values.
10701113
pub fn is_empty(&self) -> bool {
1071-
let my_callsite = self.callsite();
1072-
self.values
1073-
.iter()
1074-
.all(|(key, val)| val.is_none() || key.callsite() != my_callsite)
1114+
match self.values {
1115+
Values::All(values) => values.iter().all(|v| v.is_none()),
1116+
Values::Explicit(values) => {
1117+
let my_callsite = self.callsite();
1118+
values
1119+
.iter()
1120+
.all(|(key, val)| val.is_none() || key.callsite() != my_callsite)
1121+
}
1122+
}
10751123
}
10761124

10771125
pub(crate) fn field_set(&self) -> &FieldSet {
@@ -1081,30 +1129,17 @@ impl ValueSet<'_> {
10811129

10821130
impl fmt::Debug for ValueSet<'_> {
10831131
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1084-
self.values
1085-
.iter()
1086-
.fold(&mut f.debug_struct("ValueSet"), |dbg, (key, v)| {
1087-
if let Some(val) = v {
1088-
val.record(key, dbg);
1089-
}
1090-
dbg
1091-
})
1092-
.field("callsite", &self.callsite())
1093-
.finish()
1132+
let mut s = f.debug_struct("ValueSet");
1133+
self.record(&mut s);
1134+
s.field("callsite", &self.callsite()).finish()
10941135
}
10951136
}
10961137

10971138
impl fmt::Display for ValueSet<'_> {
10981139
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1099-
self.values
1100-
.iter()
1101-
.fold(&mut f.debug_map(), |dbg, (key, v)| {
1102-
if let Some(val) = v {
1103-
val.record(key, dbg);
1104-
}
1105-
dbg
1106-
})
1107-
.finish()
1140+
let mut s = f.debug_map();
1141+
self.record(&mut s);
1142+
s.finish()
11081143
}
11091144
}
11101145

0 commit comments

Comments
 (0)