Skip to content

Commit

Permalink
refactor(profiling): backup sample types/period
Browse files Browse the repository at this point in the history
This breaks a dependency on the string table implementation details to
be able to reset a profile and preserve the sample types and period. It
does cause more allocations, though.

Adds api::ValueType::new and uses it instead of constructing it
directly, because in most cases with `new` it becomes a single line
instead of four.
  • Loading branch information
morrisonlevi committed May 9, 2024
1 parent 47c595b commit 724f019
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 200 deletions.
12 changes: 7 additions & 5 deletions profiling-ffi/src/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,10 @@ impl<'a> TryFrom<&'a Mapping<'a>> for api::Mapping<'a> {

impl<'a> From<&'a ValueType<'a>> for api::ValueType<'a> {
fn from(vt: &'a ValueType<'a>) -> Self {
Self {
r#type: vt.type_.try_to_utf8().unwrap_or(""),
unit: vt.unit.try_to_utf8().unwrap_or(""),
}
Self::new(
vt.type_.try_to_utf8().unwrap_or(""),
vt.unit.try_to_utf8().unwrap_or(""),
)
}
}

Expand Down Expand Up @@ -693,7 +693,9 @@ pub unsafe extern "C" fn ddog_prof_Profile_serialize(
) -> SerializeResult {
(|| {
let profile = profile_ptr_to_inner(profile)?;
let old_profile = profile.reset_and_return_previous(start_time.map(SystemTime::from))?;

let start_time = start_time.map(SystemTime::from);
let old_profile = profile.reset_and_return_previous(start_time)?;
let end_time = end_time.map(SystemTime::from);
let duration = match duration_nanos {
None => None,
Expand Down
16 changes: 8 additions & 8 deletions profiling-replayer/src/replayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ impl<'pprof> Replayer<'pprof> {
) -> anyhow::Result<Vec<api::ValueType<'pprof>>> {
let mut sample_types = Vec::with_capacity(profile_index.pprof.sample_types.len());
for sample_type in profile_index.pprof.sample_types.iter() {
sample_types.push(api::ValueType {
r#type: profile_index.get_string(sample_type.r#type)?,
unit: profile_index.get_string(sample_type.unit)?,
})
sample_types.push(api::ValueType::new(
profile_index.get_string(sample_type.r#type)?,
profile_index.get_string(sample_type.unit)?,
))
}
Ok(sample_types)
}
Expand All @@ -75,10 +75,10 @@ impl<'pprof> Replayer<'pprof> {

match profile_index.pprof.period_type {
Some(period_type) => {
let r#type = api::ValueType {
r#type: profile_index.get_string(period_type.r#type)?,
unit: profile_index.get_string(period_type.unit)?,
};
let r#type = api::ValueType::new(
profile_index.get_string(period_type.r#type)?,
profile_index.get_string(period_type.unit)?,
);
Ok(Some(api::Period { r#type, value }))
}
None => Ok(None),
Expand Down
13 changes: 2 additions & 11 deletions profiling/examples/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,8 @@ use std::time::SystemTime;

// Keep this in-sync with profiles.c
fn main() {
let walltime = api::ValueType {
r#type: "wall-time",
unit: "nanoseconds",
};
let sample_types = vec![
api::ValueType {
r#type: "samples",
unit: "count",
},
walltime,
];
let walltime = api::ValueType::new("wall-time", "nanoseconds");
let sample_types = [api::ValueType::new("samples", "count"), walltime];

let period = api::Period {
r#type: walltime,
Expand Down
23 changes: 15 additions & 8 deletions profiling/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ pub struct ValueType<'a> {
pub unit: &'a str,
}

impl<'a> ValueType<'a> {
#[inline(always)]
pub fn new(r#type: &'a str, unit: &'a str) -> Self {
Self { r#type, unit }
}
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub struct Period<'a> {
pub r#type: ValueType<'a>,
Expand Down Expand Up @@ -280,21 +287,21 @@ impl<'a> TryFrom<&'a pprof::Profile> for Profile<'a> {

let period = match pprof.period_type {
Some(t) => {
let r#type = ValueType {
r#type: string_table_fetch(pprof, t.r#type)?,
unit: string_table_fetch(pprof, t.unit)?,
};
let r#type = ValueType::new(
string_table_fetch(pprof, t.r#type)?,
string_table_fetch(pprof, t.unit)?,
);
Some((pprof.period, r#type))
}
None => None,
};

let mut sample_types = Vec::with_capacity(pprof.samples.len());
for t in pprof.sample_types.iter() {
sample_types.push(ValueType {
r#type: string_table_fetch(pprof, t.r#type)?,
unit: string_table_fetch(pprof, t.unit)?,
});
sample_types.push(ValueType::new(
string_table_fetch(pprof, t.r#type)?,
string_table_fetch(pprof, t.unit)?,
));
}

let mut samples = Vec::with_capacity(pprof.samples.len());
Expand Down
2 changes: 2 additions & 0 deletions profiling/src/internal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod label;
mod location;
mod mapping;
mod observation;
mod owned_types;
mod profile;
mod sample;
mod stack_trace;
Expand All @@ -22,6 +23,7 @@ pub use label::*;
pub use location::*;
pub use mapping::*;
pub use observation::*;
pub use owned_types::*;
pub use profile::*;
pub use sample::*;
pub use stack_trace::*;
Expand Down
36 changes: 36 additions & 0 deletions profiling/src/internal/owned_types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/
// SPDX-License-Identifier: Apache-2.0

use crate::api::{Period, ValueType};

#[derive(Clone)]
pub struct OwnedValueType {
pub typ: Box<str>,
pub unit: Box<str>,
}

impl<'a> From<&'a ValueType<'a>> for OwnedValueType {
#[inline]
fn from(value_type: &'a ValueType<'a>) -> Self {
Self {
typ: String::from(value_type.r#type).into(),
unit: String::from(value_type.unit).into(),
}
}
}

#[derive(Clone)]
pub struct OwnedPeriod {
pub typ: OwnedValueType,
pub value: i64,
}

impl<'a> From<&'a Period<'a>> for OwnedPeriod {
#[inline]
fn from(period: &'a Period<'a>) -> Self {
Self {
typ: OwnedValueType::from(&period.r#type),
value: period.value,
}
}
}
Loading

0 comments on commit 724f019

Please sign in to comment.