Skip to content

Commit

Permalink
feat(ktrace): Avoid allocations for Debug fields
Browse files Browse the repository at this point in the history
Serde supports serializing `Display` values via `collect_str`, without
having to explicitly buffer them ahead of time. Using this for `Debug`
values produced by `tracing` means the ktrace protocol implementation no
longer needs to allocate.

The downside is that postcard implements `collect_str` by processing the
value twice, to determine its length. However, as
jamesmunns/postcard#32 points out, this is
likely fine since the data shouldn't be large.
  • Loading branch information
bnavetta committed Jul 11, 2022
1 parent 936a32e commit ea1a3e8
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 22 deletions.
9 changes: 6 additions & 3 deletions kernel/src/mm/root_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,13 @@ impl Builder {
})
.ok_or(Error::new(ErrorKind::InsufficientMemory))?;

tracing::debug!("Scratch space: {}", scratch.address_range());
tracing::debug!(
"Initial tracking pages: {}",
initial_tracking.address_range()
range = %scratch.address_range(),
"Scratch space",
);
tracing::debug!(
range = %initial_tracking.address_range(),
"Initial tracking pages",
);

unsafe {
Expand Down
55 changes: 36 additions & 19 deletions ktrace/proto/src/fields.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//! Serializable adapter for trace attributes

use alloc::string::String;
use alloc::vec::Vec;
use core::fmt::{Debug, Formatter, Write};
use core::fmt::{Debug, Formatter};
use phf::phf_map;
use serde::de::{Error as _, MapAccess, Visitor};
use serde::ser::{Error as _, SerializeMap};
Expand Down Expand Up @@ -51,18 +50,13 @@ impl<'a> From<&'a Event<'a>> for SerializeEvent<'a> {
}

#[inline(always)]
fn serialize_fields<S: Serializer, F: FnOnce(&mut FieldVisitor<'_, S::SerializeMap>)>(
fn serialize_fields<S: Serializer, F: FnOnce(&mut FieldVisitor<S::SerializeMap>)>(
serializer: S,
len: usize,
f: F,
) -> Result<S::Ok, S::Error> {
// Scope the temporary formatting buffer to the duration of serialization.
// Hopefully, this gives us some efficiency gains while avoiding synchronization
// and the cost of keeping buffers around.
let mut buf = String::new();

let map = serializer.serialize_map(Some(len))?;
let mut visitor = FieldVisitor::new(&mut buf, map);
let mut visitor = FieldVisitor::new(map);
f(&mut visitor);
visitor.finish()
}
Expand Down Expand Up @@ -137,7 +131,7 @@ static TYPES: phf::Map<&'static str, FieldType> = phf_map! {
"size" => FieldType::U64,
"vaddr" => FieldType::VirtualAddress,
"paddr" => FieldType::PhysicalAddress,
"test_name" => FieldType::String,
"range" => FieldType::String,
};

#[derive(Clone, Copy, Debug)]
Expand Down Expand Up @@ -176,6 +170,33 @@ impl FieldType {
}
}

fn write_debug<S: SerializeMap>(
self,
name: &str,
value: &dyn Debug,
s: &mut S,
) -> Result<(), S::Error> {
match self {
FieldType::String => {
struct SerializeDebug<'a>(&'a dyn Debug);

impl<'a> Serialize for SerializeDebug<'a> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.collect_str(&format_args!("{:?}", self.0))
}
}

s.serialize_entry(name, &SerializeDebug(value))
}
other => Err(S::Error::custom(format_args!(
"{name} value must be a {other:?}, got str"
))),
}
}

fn read<'a, M: MapAccess<'a>>(self, map: &mut M) -> Result<Value<'a>, M::Error> {
match self {
FieldType::KernelAddress => Ok(Value::KernelAddress(map.next_value()?)),
Expand All @@ -187,18 +208,16 @@ impl FieldType {
}
}

struct FieldVisitor<'a, S: SerializeMap> {
struct FieldVisitor<S: SerializeMap> {
state: Result<(), S::Error>,
serializer: S,
buf: &'a mut String,
}

impl<'a, S: SerializeMap> FieldVisitor<'a, S> {
fn new(buf: &'a mut String, serializer: S) -> Self {
impl<S: SerializeMap> FieldVisitor<S> {
fn new(serializer: S) -> Self {
Self {
state: Ok(()),
serializer,
buf,
}
}

Expand All @@ -208,14 +227,12 @@ impl<'a, S: SerializeMap> FieldVisitor<'a, S> {
}
}

impl<'a, S: SerializeMap> Visit for FieldVisitor<'a, S> {
impl<S: SerializeMap> Visit for FieldVisitor<S> {
fn record_debug(&mut self, field: &Field, value: &dyn Debug) {
if self.state.is_ok() {
let name = field.name();
if let Some(ty) = TYPES.get(name) {
write!(&mut self.buf, "{value:?}").unwrap();
self.state = ty.write_str(field.name(), self.buf, &mut self.serializer);
self.buf.clear();
self.state = ty.write_debug(field.name(), value, &mut self.serializer);
} else {
panic!("unknown field: {field}");
}
Expand Down

0 comments on commit ea1a3e8

Please sign in to comment.