Skip to content

Commit

Permalink
WIPvariance
Browse files Browse the repository at this point in the history
  • Loading branch information
@douglas-raillard-arm
douglas-raillard-arm committed Nov 14, 2023
1 parent 61846f4 commit b74c35d
Showing 1 changed file with 28 additions and 44 deletions.
72 changes: 28 additions & 44 deletions tools/analysis/traceevent/src/buffer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,14 @@ use crate::{
// Keep a BTreeMap for fast lookup and to avoid huge Vec allocation in case
// some event ids are very large. Since most traces will contain just a few
// types of events, build up the smallest mapping as it goes.
struct EventDescMap<T, InitDescF> {
// We use pointers instead of &'h here as EventDescMap is held behind an &'edm mut in
// EventVisitor, which would make it invariant in 'h, but we want to stay covariant.
header: *const Header,
cold_map: BTreeMap<EventId, *const EventDesc>,
hot_map: BTreeMap<EventId, (*const EventDesc, T)>,
struct EventDescMap<'h, T, InitDescF> {
header: &'h Header,
cold_map: BTreeMap<EventId, &'h EventDesc>,
hot_map: BTreeMap<EventId, (&'h EventDesc, T)>,
init_desc: Arc<Mutex<InitDescF>>,
}

impl<T: Debug, InitDescF> Debug for EventDescMap<T, InitDescF> {
impl<'h, T: Debug, InitDescF> Debug for EventDescMap<'h, T, InitDescF> {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), core::fmt::Error> {
f.debug_struct("EventDescMap")
.field("cold_map", &self.cold_map)
Expand All @@ -55,7 +53,7 @@ impl<T: Debug, InitDescF> Debug for EventDescMap<T, InitDescF> {
}
}

impl<'h, T, InitDescF> EventDescMap<T, InitDescF>
impl<'h, T, InitDescF> EventDescMap<'h, T, InitDescF>
where
InitDescF: FnMut(&'h Header, &'h EventDesc) -> T + 'h,
{
Expand All @@ -65,47 +63,24 @@ where
cold_map: header
.event_descs()
.into_iter()
.map(|desc| (desc.id, desc as *const EventDesc))
.map(|desc| (desc.id, desc))
.collect(),
hot_map: BTreeMap::new(),
init_desc,
}
}
#[inline]
fn lookup<'edm>(&'edm mut self, id: EventId) -> Option<(&'h EventDesc, &'edm T)> {
macro_rules! as_h_ref {
($ptr:expr, $ty:ty) => {{
let ptr: *const $ty = $ptr;
// SAFETY: We created these pointers from &'h Header in the first place so:
// * The lifetime 'h is appropriate for values contained in an &'h Header
// * EventVisitor is holding a reference &'h Header, so the values will not get
// de-allocated under our feet.
// * There is no aliasing issue since we only manipulate shared references and
// const pointers.

// TODO: Creating a &'h means we are creating an unbounded lifetime here, since 'h
// is not contained in the type. It could therefore differ between the call to
// new() and lookup(), so we fully rely on the caller doing something sensible.
// However, we removed 'h from the type in order to make &'edm mut EventDescMap<...>>
// covariant in 'h so we cannot just add it back.
let val: &'h $ty = unsafe { &*ptr };
val
}}
}
match self.hot_map.entry(id) {
Entry::Occupied(entry) => {
let (desc, user) = entry.into_mut();
let desc = as_h_ref!(*desc, EventDesc);
Some((desc, user))
Some((*desc, user))
}
Entry::Vacant(entry) => match self.cold_map.remove(&id) {
Some(desc) => {
let mut init_desc = self.init_desc.lock().unwrap();
let header = as_h_ref!(self.header, Header);
let desc = as_h_ref!(desc, EventDesc);
let (desc, user) = entry.insert((desc, init_desc(header, desc)));
let desc = as_h_ref!(*desc, EventDesc);
Some((desc, user))
let (desc, user) = entry.insert((desc, init_desc(self.header, desc)));
Some((*desc, user))
}
None => None,
},
Expand All @@ -120,10 +95,11 @@ pub struct EventVisitor<'i, 'h, 'edm, InitDescF, T = ()> {
pub timestamp: Timestamp,
pub buffer_id: &'h BufferId,

// Using &'edm mut here means EventVisitor is invariant in any lifetime contained in T.
// Using *mut here means EventVisitor is invariant in any lifetime contained in T.
// However, the only values we store in the EventDescMap are either owned by it or have a
// longer lifetime ('h outlives 'edm), so it's sound to be covariant in 'edm.
desc_map: *mut EventDescMap<T, InitDescF>,
// So in practice we use 'static but then we cast back to 'h.
desc_map: *mut EventDescMap<'static, T, InitDescF>,
_phantom_desc_map: PhantomData<&'edm mut ()>,

scratch: &'i ScratchAlloc,
Expand All @@ -146,8 +122,17 @@ impl<'i, 'h, 'edm, InitDescF, T> EventVisitor<'i, 'h, 'edm, InitDescF, T> {
timestamp: Timestamp,
data: &'i [u8],
scratch: &'i ScratchAlloc,
desc_map: &'edm mut EventDescMap<T, InitDescF>,
desc_map: &'edm mut EventDescMap<'h, T, InitDescF>,
) -> Self {

let desc_map: &'edm mut EventDescMap<'h, T, InitDescF> = desc_map;
// SAFETY: Erase the lifetime 'h and replace by 'static so that we stay covariant in 'h. We
// won't be using the desc_map reference past 'h since:
// * 'h outlives 'edm
// * we don't leak self.desc_map anywhere without attaching the 'edm lifetime to what was
// leaked
let desc_map: &'edm mut EventDescMap<'static, T, InitDescF> = unsafe {core::mem::transmute(desc_map)};

EventVisitor {
data,
header,
Expand Down Expand Up @@ -277,15 +262,14 @@ where
Ok(&desc.name)
}

fn desc_map(&self) -> &'edm mut EventDescMap<T, InitDescF> {
fn desc_map(&self) -> &'edm mut EventDescMap<'h, T, InitDescF> {
// SAFETY: This comes from an &'edm mut reference in the first place, and since
// EventVisitor::new() requires an &'edm mut EventDescMap, we cannot accidentally
// borrow it mutably more than once. This makes it safe to turn it back to an &'edm
// mut.
let desc_map = self.desc_map;
let desc_map: &'edm mut EventDescMap<T, InitDescF> = unsafe {
&mut *desc_map
};
let desc_map: *mut EventDescMap<'static, T, InitDescF> = self.desc_map;
let desc_map: &'edm mut EventDescMap<'static, T, InitDescF> = unsafe { &mut *desc_map };
let desc_map: &'edm mut EventDescMap<'h, T, InitDescF> = unsafe{core::mem::transmute(desc_map)};
desc_map
}

Expand Down Expand Up @@ -660,7 +644,7 @@ struct BufferItem<'a, T, InitDescF>(
Result<
(
&'a Header,
&'a mut EventDescMap<T, InitDescF>,
&'a mut EventDescMap<'a, T, InitDescF>,
&'a BufferId,
Timestamp,
&'a [u8],
Expand Down

0 comments on commit b74c35d

Please sign in to comment.