From 25e1220e41491a09e3101b2d04f830a518d3cdf6 Mon Sep 17 00:00:00 2001 From: Sean Lynch Date: Sun, 14 May 2023 13:54:52 -0700 Subject: [PATCH 1/3] Change Read record to only support non-group reads As it turns out, PERF_RECORD_READ events are only emitted when the inherit and inherit_stat flags are set. inherit is incompatible with READ_FORMAT_GROUP so PERF_RECORD_READ will never be emitted with a group format. This doesn't mean we can delete all the code around reading group data because they are still used within PERF_RECORD_SAMPLE. --- src/records/mod.rs | 6 +++--- src/records/read.rs | 43 +++++++++++++++++-------------------------- src/visitor.rs | 2 +- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/records/mod.rs b/src/records/mod.rs index bdfab0f..45fb34c 100644 --- a/src/records/mod.rs +++ b/src/records/mod.rs @@ -187,7 +187,7 @@ pub enum Record<'a> { Throttle(Throttle), Unthrottle(Throttle), Fork(Fork), - Read(Read<'a>), + Read(Read), Sample(Box>), Mmap2(Mmap2<'a>), Aux(Aux), @@ -241,7 +241,7 @@ record_from!(Comm<'a>); // These are both the same struct // record_from!(Exit); // record_from!(Fork); -record_from!(Read<'a>); +record_from!(Read); record_from!(Mmap2<'a>); record_from!(Aux); record_from!(ITraceStart); @@ -300,7 +300,7 @@ impl<'a> crate::Visitor<'a> for RecordVisitor { Record::Fork(record) } - fn visit_read(self, record: Read<'a>, _: crate::RecordMetadata) -> Self::Output { + fn visit_read(self, record: Read, _: crate::RecordMetadata) -> Self::Output { record.into() } diff --git a/src/records/read.rs b/src/records/read.rs index 97cda23..cb649d4 100644 --- a/src/records/read.rs +++ b/src/records/read.rs @@ -15,24 +15,15 @@ use std::iter::FusedIterator; /// /// [manpage]: http://man7.org/linux/man-pages/man2/perf_event_open.2.html #[derive(Clone, Debug)] -pub struct Read<'a> { +pub struct Read { /// The process ID. pub pid: u32, /// The thread ID. pub tid: u32, - pub values: ReadData<'a>, -} - -impl<'a> Read<'a> { - /// Convert all the borrowed data in this `Read` into owned data. - pub fn into_owned(self) -> Read<'static> { - Read { - values: self.values.into_owned(), - ..self - } - } + /// The value read from the counter during task switch. + pub values: ReadValue, } #[derive(Clone, Debug)] @@ -41,10 +32,10 @@ pub enum ReadData<'a> { /// /// This is what will be generated if the [`ParseConfig`]'s `read_format` /// did not contain `READ_FORMAT_GROUP`. - Single(SingleRead), + Single(ReadValue), /// Data for all counters in a group. - Group(GroupRead<'a>), + Group(ReadGroup<'a>), } impl<'a> ReadData<'a> { @@ -76,7 +67,7 @@ impl<'a> ReadData<'a> { } #[derive(Clone)] -pub struct SingleRead { +pub struct ReadValue { read_format: ReadFormat, value: u64, time_enabled: u64, @@ -85,7 +76,7 @@ pub struct SingleRead { lost: u64, } -impl SingleRead { +impl ReadValue { /// The value of the counter. pub fn value(&self) -> u64 { self.value @@ -121,7 +112,7 @@ impl SingleRead { } } -impl fmt::Debug for SingleRead { +impl fmt::Debug for ReadValue { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let read_format = self.read_format; let mut dbg = debug_if! { @@ -140,14 +131,14 @@ impl fmt::Debug for SingleRead { } #[derive(Clone)] -pub struct GroupRead<'a> { +pub struct ReadGroup<'a> { read_format: ReadFormat, time_enabled: u64, time_running: u64, data: Cow<'a, [u64]>, } -impl<'a> GroupRead<'a> { +impl<'a> ReadGroup<'a> { pub fn len(&self) -> usize { self.data.len() / self.read_format.element_len() } @@ -156,8 +147,8 @@ impl<'a> GroupRead<'a> { self.len() == 0 } - pub fn into_owned(self) -> GroupRead<'static> { - GroupRead { + pub fn into_owned(self) -> ReadGroup<'static> { + ReadGroup { data: self.data.into_owned().into(), ..self } @@ -186,7 +177,7 @@ impl<'a> GroupRead<'a> { } } -impl fmt::Debug for GroupRead<'_> { +impl fmt::Debug for ReadGroup<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { struct Entries<'a>(GroupIter<'a>); @@ -280,7 +271,7 @@ pub struct GroupIter<'a> { } impl<'a> GroupIter<'a> { - fn new(group: &'a GroupRead) -> Self { + fn new(group: &'a ReadGroup) -> Self { let read_format = group.read_format; Self { @@ -328,7 +319,7 @@ impl<'a> ExactSizeIterator for GroupIter<'a> {} impl<'a> FusedIterator for GroupIter<'a> {} -impl<'p> Parse<'p> for SingleRead { +impl<'p> Parse<'p> for ReadValue { fn parse(p: &mut Parser) -> ParseResult where E: Endian, @@ -369,7 +360,7 @@ impl<'p> Parse<'p> for SingleRead { } } -impl<'p> Parse<'p> for GroupRead<'p> { +impl<'p> Parse<'p> for ReadGroup<'p> { fn parse(p: &mut Parser) -> ParseResult where E: Endian, @@ -435,7 +426,7 @@ impl<'p> Parse<'p> for ReadData<'p> { } } -impl<'p> Parse<'p> for Read<'p> { +impl<'p> Parse<'p> for Read { fn parse(p: &mut Parser) -> ParseResult where E: Endian, diff --git a/src/visitor.rs b/src/visitor.rs index e2c189f..430f913 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -122,7 +122,7 @@ pub trait Visitor<'a>: Sized { } /// Visit a [`Read`] record. - fn visit_read(self, record: Read<'a>, metadata: RecordMetadata) -> Self::Output { + fn visit_read(self, record: Read, metadata: RecordMetadata) -> Self::Output { self.visit_unimplemented(metadata) } From 70a5b0f00f36b43138b94d7c79c9e4d825737103 Mon Sep 17 00:00:00 2001 From: Sean Lynch Date: Sun, 14 May 2023 14:39:21 -0700 Subject: [PATCH 2/3] Document ReadValue --- src/records/read.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/records/read.rs b/src/records/read.rs index cb649d4..99691e9 100644 --- a/src/records/read.rs +++ b/src/records/read.rs @@ -66,6 +66,7 @@ impl<'a> ReadData<'a> { } } +/// Data read from a counter. #[derive(Clone)] pub struct ReadValue { read_format: ReadFormat, From 623ab38aa1354efc15e7b06b27a23670492bcb6f Mon Sep 17 00:00:00 2001 From: Sean Lynch Date: Sun, 14 May 2023 15:03:36 -0700 Subject: [PATCH 3/3] Rework how reading read_format values works - Sample now always contains a ReadGroup and converts if the source read_format is only for a single value. - Read now only contains a ReadValue since it is not possible to create a Read record when GROUP is set in read_format - ReadGroup can now be converted into ReadValue via TryFrom - ReadValue can now be converted into ReadGroup via From - ReadData has been deleted since the conversions take its place --- src/records/read.rs | 145 ++++++++++++++++++++++++------------------ src/records/sample.rs | 15 +++-- 2 files changed, 93 insertions(+), 67 deletions(-) diff --git a/src/records/read.rs b/src/records/read.rs index 99691e9..2d1c942 100644 --- a/src/records/read.rs +++ b/src/records/read.rs @@ -1,5 +1,3 @@ -#![allow(missing_docs)] - use crate::error::ParseError; use crate::prelude::*; use std::borrow::Cow; @@ -26,46 +24,6 @@ pub struct Read { pub values: ReadValue, } -#[derive(Clone, Debug)] -pub enum ReadData<'a> { - /// Data for only a single counter. - /// - /// This is what will be generated if the [`ParseConfig`]'s `read_format` - /// did not contain `READ_FORMAT_GROUP`. - Single(ReadValue), - - /// Data for all counters in a group. - Group(ReadGroup<'a>), -} - -impl<'a> ReadData<'a> { - pub fn into_owned(self) -> ReadData<'static> { - match self { - Self::Single(data) => ReadData::Single(data), - Self::Group(data) => ReadData::Group(data.into_owned()), - } - } - - /// The duration for which this event was enabled, in nanoseconds. - pub fn time_enabled(&self) -> Option { - match self { - Self::Single(data) => data.time_enabled(), - Self::Group(data) => data.time_enabled(), - } - } - - /// The duration for which this event was running, in nanoseconds. - /// - /// This will be less than `time_enabled` if the kernel ended up having to - /// multiplex multiple counters on the CPU. - pub fn time_running(&self) -> Option { - match self { - Self::Single(data) => data.time_running(), - Self::Group(data) => data.time_running(), - } - } -} - /// Data read from a counter. #[derive(Clone)] pub struct ReadValue { @@ -113,6 +71,28 @@ impl ReadValue { } } +impl TryFrom> for ReadValue { + type Error = TryFromGroupError; + + fn try_from(value: ReadGroup<'_>) -> Result { + let mut entries = value.entries(); + let entry = entries.next().ok_or(TryFromGroupError(()))?; + + if entries.next().is_some() { + return Err(TryFromGroupError(())); + } + + Ok(Self { + read_format: value.read_format - ReadFormat::GROUP, + value: entry.value(), + time_enabled: value.time_enabled, + time_running: value.time_running, + id: entry.id, + lost: entry.lost, + }) + } +} + impl fmt::Debug for ReadValue { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let read_format = self.read_format; @@ -131,6 +111,7 @@ impl fmt::Debug for ReadValue { } } +/// The values read from a group of counters. #[derive(Clone)] pub struct ReadGroup<'a> { read_format: ReadFormat, @@ -140,14 +121,17 @@ pub struct ReadGroup<'a> { } impl<'a> ReadGroup<'a> { + /// The number of counters contained within this group. pub fn len(&self) -> usize { - self.data.len() / self.read_format.element_len() + self.entries().count() } + /// Whether this group has any counters at all. pub fn is_empty(&self) -> bool { self.len() == 0 } + /// Convert all the borrowed data in this `ReadGroup` into owned data. pub fn into_owned(self) -> ReadGroup<'static> { ReadGroup { data: self.data.into_owned().into(), @@ -172,12 +156,42 @@ impl<'a> ReadGroup<'a> { .then_some(self.time_running) } + /// Get a group entry by its index. + pub fn get(&self, index: usize) -> Option { + self.entries().nth(index) + } + + /// Get a group entry by its counter id. + pub fn get_by_id(&self, id: u64) -> Option { + if !self.read_format.contains(ReadFormat::ID) { + return None; + } + + self.entries().find(|entry| entry.id() == Some(id)) + } + /// Iterate over the entries contained within this `GroupRead`. pub fn entries(&self) -> GroupIter { GroupIter::new(self) } } +impl<'a> From for ReadGroup<'a> { + fn from(value: ReadValue) -> Self { + let mut data = Vec::with_capacity(3); + data.push(value.value()); + data.extend(value.id()); + data.extend(value.lost()); + + Self { + read_format: value.read_format | ReadFormat::GROUP, + time_enabled: value.time_enabled, + time_running: value.time_running, + data: Cow::Owned(data), + } + } +} + impl fmt::Debug for ReadGroup<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { struct Entries<'a>(GroupIter<'a>); @@ -201,6 +215,10 @@ impl fmt::Debug for ReadGroup<'_> { } } +/// The values read from a single perf event counter. +/// +/// This will always include the counter value. The other fields are optional +/// depending on how the counter's `read_format` was configured. #[derive(Copy, Clone)] pub struct GroupEntry { read_format: ReadFormat, @@ -231,7 +249,7 @@ impl GroupEntry { let mut iter = slice.iter().copied(); let mut read = || { iter.next() - .expect("slice was not the corred size for the configured read_format") + .expect("slice was not the correct size for the configured read_format") }; Self { @@ -267,7 +285,7 @@ impl fmt::Debug for GroupEntry { /// See [`GroupRead::entries`]. #[derive(Clone)] pub struct GroupIter<'a> { - iter: std::slice::Chunks<'a, u64>, + iter: std::slice::ChunksExact<'a, u64>, read_format: ReadFormat, } @@ -276,7 +294,7 @@ impl<'a> GroupIter<'a> { let read_format = group.read_format; Self { - iter: group.data.chunks(read_format.element_len()), + iter: group.data.chunks_exact(read_format.element_len()), read_format, } } @@ -316,7 +334,12 @@ impl<'a> DoubleEndedIterator for GroupIter<'a> { } } -impl<'a> ExactSizeIterator for GroupIter<'a> {} +impl<'a> ExactSizeIterator for GroupIter<'a> { + #[inline] + fn len(&self) -> usize { + self.iter.len() + } +} impl<'a> FusedIterator for GroupIter<'a> {} @@ -411,22 +434,6 @@ impl<'p> Parse<'p> for ReadGroup<'p> { } } -impl<'p> Parse<'p> for ReadData<'p> { - fn parse(p: &mut Parser) -> ParseResult - where - E: Endian, - B: ParseBuf<'p>, - { - let read_format = p.config().read_format(); - - if read_format.contains(ReadFormat::GROUP) { - Ok(Self::Group(p.parse()?)) - } else { - Ok(Self::Single(p.parse()?)) - } - } -} - impl<'p> Parse<'p> for Read { fn parse(p: &mut Parser) -> ParseResult where @@ -440,3 +447,15 @@ impl<'p> Parse<'p> for Read { }) } } + +/// Error when attempting to convert [`ReadGroup`] to a [`ReadValue`]. +#[derive(Clone, Debug)] +pub struct TryFromGroupError(()); + +impl fmt::Display for TryFromGroupError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("can only convert groups with a single element to ReadValues") + } +} + +impl std::error::Error for TryFromGroupError {} diff --git a/src/records/sample.rs b/src/records/sample.rs index 1bbdb0a..0a3d3a9 100644 --- a/src/records/sample.rs +++ b/src/records/sample.rs @@ -11,7 +11,8 @@ use perf_event_open_sys::bindings::perf_mem_data_src; use crate::parse::ParseError; use crate::prelude::*; -use crate::ReadData; +use crate::ReadGroup; +use crate::ReadValue; mod sample_impl { use super::*; @@ -31,7 +32,7 @@ mod sample_impl { pub stream_id: u64, pub cpu: u32, pub period: u64, - pub values: ReadData<'a>, + pub values: ReadGroup<'a>, pub callchain: Cow<'a, [u64]>, pub raw: Cow<'a, [u8]>, pub lbr_hw_index: u64, @@ -97,7 +98,7 @@ impl<'a> Sample<'a> { self.0.period().copied() } - pub fn values(&self) -> Option<&ReadData<'a>> { + pub fn values(&self) -> Option<&ReadGroup<'a>> { self.0.values() } @@ -180,7 +181,13 @@ impl<'p> Parse<'p> for Sample<'p> { Ok((p.parse_u32()?, p.parse_u32()?).0) })?; let period = p.parse_if(sty.contains(SampleFlags::PERIOD))?; - let values = p.parse_if(sty.contains(SampleFlags::READ))?; + let values = p.parse_if_with(sty.contains(SampleFlags::READ), |p| { + if p.config().read_format().contains(ReadFormat::GROUP) { + p.parse() + } else { + ReadValue::parse(p).map(From::from) + } + })?; let callchain = p.parse_if_with(sty.contains(SampleFlags::CALLCHAIN), |p| { let nr = p.parse_u64()? as _; unsafe { p.parse_slice(nr) }