Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscriber: don't use SmallVecs for filter fields #1568

Merged
merged 2 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions tracing-subscriber/src/filter/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ pub struct ParseError {
#[derive(Debug, PartialEq, Eq, Clone)]
pub(crate) struct StaticDirective {
pub(in crate::filter) target: Option<String>,
pub(in crate::filter) field_names: FilterVec<String>,
pub(in crate::filter) field_names: Vec<String>,
pub(in crate::filter) level: LevelFilter,
}

#[cfg(feature = "smallvec")]
pub(in crate::filter) type FilterVec<T> = smallvec::SmallVec<[T; 8]>;
pub(crate) type FilterVec<T> = smallvec::SmallVec<[T; 8]>;
#[cfg(not(feature = "smallvec"))]
pub(in crate::filter) type FilterVec<T> = Vec<T>;
pub(crate) type FilterVec<T> = Vec<T>;

#[derive(Debug, PartialEq, Clone)]
pub(in crate::filter) struct DirectiveSet<T> {
Expand Down Expand Up @@ -129,7 +129,7 @@ impl DirectiveSet<StaticDirective> {
impl StaticDirective {
pub(in crate::filter) fn new(
target: Option<String>,
field_names: FilterVec<String>,
field_names: Vec<String>,
level: LevelFilter,
) -> Self {
Self {
Expand Down Expand Up @@ -221,7 +221,7 @@ impl Default for StaticDirective {
fn default() -> Self {
StaticDirective {
target: None,
field_names: FilterVec::new(),
field_names: Vec::new(),
level: LevelFilter::ERROR,
}
}
Expand Down Expand Up @@ -288,7 +288,7 @@ impl FromStr for StaticDirective {

let mut split = part0.split("[{");
let target = split.next().map(String::from);
let mut field_names = FilterVec::new();
let mut field_names = Vec::new();
// Directive includes fields:
// * `foo[{bar}]=trace`
// * `foo[{bar,baz}]=trace`
Expand Down Expand Up @@ -326,12 +326,12 @@ impl FromStr for StaticDirective {
Ok(level) => Self {
level,
target: None,
field_names: FilterVec::new(),
field_names: Vec::new(),
},
Err(_) => Self {
target: Some(String::from(part0)),
level: LevelFilter::TRACE,
field_names: FilterVec::new(),
field_names: Vec::new(),
},
})
}
Expand Down
13 changes: 6 additions & 7 deletions tracing-subscriber/src/filter/env/directive.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use super::FilterVec;
pub(crate) use crate::filter::directive::{ParseError, StaticDirective};
pub(crate) use crate::filter::directive::{FilterVec, ParseError, StaticDirective};
use crate::filter::{
directive::{DirectiveSet, Match},
env::{field, FieldMap},
Expand All @@ -16,7 +15,7 @@ use tracing_core::{span, Level, Metadata};
#[cfg_attr(docsrs, doc(cfg(feature = "env-filter")))]
pub struct Directive {
in_span: Option<String>,
fields: FilterVec<field::Match>,
fields: Vec<field::Match>,
pub(crate) target: Option<String>,
pub(crate) level: LevelFilter,
}
Expand Down Expand Up @@ -216,12 +215,12 @@ impl FromStr for Directive {
FIELD_FILTER_RE
.find_iter(c.as_str())
.map(|c| c.as_str().parse())
.collect::<Result<FilterVec<_>, _>>()
.collect::<Result<Vec<_>, _>>()
})
.unwrap_or_else(|| Ok(FilterVec::new()));
.unwrap_or_else(|| Ok(Vec::new()));
Some((span, fields))
})
.unwrap_or_else(|| (None, Ok(FilterVec::new())));
.unwrap_or_else(|| (None, Ok(Vec::new())));

let level = caps
.name("level")
Expand All @@ -244,7 +243,7 @@ impl Default for Directive {
level: LevelFilter::OFF,
target: None,
in_span: None,
fields: FilterVec::new(),
fields: Vec::new(),
}
}
}
Expand Down
34 changes: 29 additions & 5 deletions tracing-subscriber/src/filter/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,6 @@ thread_local! {

type FieldMap<T> = HashMap<Field, T>;

#[cfg(feature = "smallvec")]
type FilterVec<T> = smallvec::SmallVec<[T; 8]>;
#[cfg(not(feature = "smallvec"))]
type FilterVec<T> = Vec<T>;

/// Indicates that an error occurred while parsing a `EnvFilter` from an
/// environment variable.
#[cfg_attr(docsrs, doc(cfg(feature = "env-filter")))]
Expand Down Expand Up @@ -711,4 +706,33 @@ mod tests {
assert_eq!(f1.statics, f2.statics);
assert_eq!(f1.dynamics, f2.dynamics);
}

#[test]
fn size_of_filters() {
fn print_sz(s: &str) {
let filter = s.parse::<EnvFilter>().expect("filter should parse");
println!(
"size_of_val({:?})\n -> {}B",
s,
std::mem::size_of_val(&filter)
);
}

print_sz("info");

print_sz("foo=debug");

print_sz(
"crate1::mod1=error,crate1::mod2=warn,crate1::mod2::mod3=info,\
crate2=debug,crate3=trace,crate3::mod2::mod1=off",
);

print_sz("[span1{foo=1}]=error,[span2{bar=2 baz=false}],crate2[{quux=\"quuux\"}]=debug");

print_sz(
"crate1::mod1=error,crate1::mod2=warn,crate1::mod2::mod3=info,\
crate2=debug,crate3=trace,crate3::mod2::mod1=off,[span1{foo=1}]=error,\
[span2{bar=2 baz=false}],crate2[{quux=\"quuux\"}]=debug",
);
}
}
45 changes: 33 additions & 12 deletions tracing-subscriber/src/filter/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,11 @@ mod tests {
assert_eq!(dirs.len(), 2, "\nparsed: {:#?}", dirs);
assert_eq!(dirs[0].target, Some("server".to_string()));
assert_eq!(dirs[0].level, LevelFilter::DEBUG);
assert_eq!(dirs[0].field_names, FilterVec::<String>::default());
assert_eq!(dirs[0].field_names, Vec::<String>::new());

assert_eq!(dirs[1].target, Some("common".to_string()));
assert_eq!(dirs[1].level, LevelFilter::INFO);
assert_eq!(dirs[1].field_names, FilterVec::<String>::default());
assert_eq!(dirs[1].field_names, Vec::<String>::new());
}

fn expect_parse_level_directives(s: &str) {
Expand All @@ -374,27 +374,27 @@ mod tests {

assert_eq!(dirs[0].target, Some("crate3::mod2::mod1".to_string()));
assert_eq!(dirs[0].level, LevelFilter::OFF);
assert_eq!(dirs[0].field_names, FilterVec::<String>::default());
assert_eq!(dirs[0].field_names, Vec::<String>::new());

assert_eq!(dirs[1].target, Some("crate1::mod2::mod3".to_string()));
assert_eq!(dirs[1].level, LevelFilter::INFO);
assert_eq!(dirs[1].field_names, FilterVec::<String>::default());
assert_eq!(dirs[1].field_names, Vec::<String>::new());

assert_eq!(dirs[2].target, Some("crate1::mod2".to_string()));
assert_eq!(dirs[2].level, LevelFilter::WARN);
assert_eq!(dirs[2].field_names, FilterVec::<String>::default());
assert_eq!(dirs[2].field_names, Vec::<String>::new());

assert_eq!(dirs[3].target, Some("crate1::mod1".to_string()));
assert_eq!(dirs[3].level, LevelFilter::ERROR);
assert_eq!(dirs[3].field_names, FilterVec::<String>::default());
assert_eq!(dirs[3].field_names, Vec::<String>::new());

assert_eq!(dirs[4].target, Some("crate3".to_string()));
assert_eq!(dirs[4].level, LevelFilter::TRACE);
assert_eq!(dirs[4].field_names, FilterVec::<String>::default());
assert_eq!(dirs[4].field_names, Vec::<String>::new());

assert_eq!(dirs[5].target, Some("crate2".to_string()));
assert_eq!(dirs[5].level, LevelFilter::DEBUG);
assert_eq!(dirs[5].field_names, FilterVec::<String>::default());
assert_eq!(dirs[5].field_names, Vec::<String>::new());
}

#[test]
Expand All @@ -418,19 +418,19 @@ mod tests {
assert_eq!(dirs.len(), 4, "\nparsed: {:#?}", dirs);
assert_eq!(dirs[0].target, Some("crate1::mod2".to_string()));
assert_eq!(dirs[0].level, LevelFilter::TRACE);
assert_eq!(dirs[0].field_names, FilterVec::<String>::default());
assert_eq!(dirs[0].field_names, Vec::<String>::new());

assert_eq!(dirs[1].target, Some("crate1::mod1".to_string()));
assert_eq!(dirs[1].level, LevelFilter::ERROR);
assert_eq!(dirs[1].field_names, FilterVec::<String>::default());
assert_eq!(dirs[1].field_names, Vec::<String>::new());

assert_eq!(dirs[2].target, Some("crate3".to_string()));
assert_eq!(dirs[2].level, LevelFilter::OFF);
assert_eq!(dirs[2].field_names, FilterVec::<String>::default());
assert_eq!(dirs[2].field_names, Vec::<String>::new());

assert_eq!(dirs[3].target, Some("crate2".to_string()));
assert_eq!(dirs[3].level, LevelFilter::DEBUG);
assert_eq!(dirs[3].field_names, FilterVec::<String>::default());
assert_eq!(dirs[3].field_names, Vec::<String>::new());
}

#[test]
Expand All @@ -456,4 +456,25 @@ mod tests {
crate3=5,crate3::mod2::mod1=0",
)
}

#[test]
fn size_of_filters() {
fn print_sz(s: &str) {
let filter = s.parse::<Targets>().expect("filter should parse");
println!(
"size_of_val({:?})\n -> {}B",
s,
std::mem::size_of_val(&filter)
);
}

print_sz("info");

print_sz("foo=debug");

print_sz(
"crate1::mod1=error,crate1::mod2=warn,crate1::mod2::mod3=info,\
crate2=debug,crate3=trace,crate3::mod2::mod1=off",
);
}
}