refactor(profiling): use libdd-profiling's ThinStr (#3631)

Author: morrisonlevi

profiling/src/lib.rs

@@ -7,7 +7,6 @@ pub mod module_globals;
 pub mod profiling;
 mod pthread;
 mod sapi;
-mod thin_str;
 mod wall_time;
 
 #[cfg(php_run_time_cache)]

profiling/src/profiling/stack_walking.rs

@@ -162,11 +162,11 @@ unsafe fn extract_file_and_line(
 mod detail {
     use super::*;
     use crate::string_set::StringSet;
-    use crate::thin_str::ThinStr;
     use crate::{RefCellExt, RefCellExtError};
+    use libdd_profiling::profiles::collections::ThinStr;
     use log::{debug, trace};
     use std::cell::RefCell;
-    use std::ptr::NonNull;
+    use std::ffi::c_void;
 
     struct StringCache<'a> {
         /// Refers to a function's run time cache reserved by this extension.
@@ -188,23 +188,21 @@ mod detail {
             debug_assert!(slot < self.cache_slots.len());
             let cached = unsafe { self.cache_slots.get_unchecked_mut(slot) };
 
-            let ptr = *cached as *mut u8;
-            match NonNull::new(ptr) {
+            let ptr = *cached as *mut c_void;
+            match std::ptr::NonNull::new(ptr) {
                 Some(non_null) => {
-                    // SAFETY: transmuting ThinStr from its repr.
-                    let thin_str: ThinStr = unsafe { core::mem::transmute(non_null) };
-                    // SAFETY: the string set is only reset between requests,
-                    // so this ThinStr points into the same string set that
-                    // created it.
+                    // SAFETY: the raw pointer was produced by ThinStr::into_raw
+                    // and the string set (which owns the backing memory) is
+                    // still alive because it is only reset between requests.
+                    let thin_str: ThinStr = unsafe { ThinStr::from_raw(non_null) };
                     let str = unsafe { self.string_set.get_thin_str(thin_str) };
                     Some(str.to_string())
                 }
                 None => {
                     let string = f()?;
                     let thin_str = self.string_set.insert(&string);
-                    // SAFETY: transmuting ThinStr into its repr.
-                    let non_null: NonNull<u8> = unsafe { core::mem::transmute(thin_str) };
-                    *cached = non_null.as_ptr() as usize;
+                    let raw = thin_str.into_raw();
+                    *cached = raw.as_ptr() as usize;
                     Some(string)
                 }
             }

profiling/src/string_set.rs

@@ -1,11 +1,24 @@
-use crate::thin_str::ThinStr;
 use core::hash;
 use core::ops::Deref;
-use libdd_alloc::{ChainAllocator, VirtualAllocator};
+use libdd_alloc::{AllocError, Allocator, ChainAllocator, VirtualAllocator};
+use libdd_profiling::profiles::collections::ThinStr;
 
 type Hasher = hash::BuildHasherDefault<rustc_hash::FxHasher>;
 type HashSet<K> = std::collections::HashSet<K, Hasher>;
 
+/// Allocates and constructs a [`ThinStr`] in one step.
+///
+/// This combines [`ThinStr::try_allocate_for`] and [`ThinStr::try_from_str_in`]
+/// for convenience. The returned [`ThinStr`] borrows from the allocation made
+/// by `alloc`, so the allocator must outlive the returned reference.
+fn try_new_thin_str_in<'a, A: Allocator>(s: &str, alloc: &'a A) -> Result<ThinStr<'a>, AllocError> {
+    let obj = ThinStr::try_allocate_for(s, alloc)?;
+    // SAFETY: `obj` was just allocated by us, no other references exist,
+    // so we can safely create a `&mut [MaybeUninit<u8>]` from it.
+    let uninit = unsafe { &mut *obj.as_ptr() };
+    ThinStr::try_from_str_in(s, uninit)
+}
+
 /// Holds unique strings and provides [StringId]s that correspond to the order
 /// that the strings were inserted.
 pub struct StringSet {
@@ -77,7 +90,7 @@ impl StringSet {
                 // No match. Make a new string in the arena, and fudge its
                 // lifetime to appease the borrow checker.
                 let new_str = {
-                    let s = ThinStr::try_from_str_in(str, &self.arena)
+                    let s = try_new_thin_str_in(str, &self.arena)
                         .expect("allocation for StringSet::insert to succeed");
 
                     // SAFETY: all references to this value get re-narrowed to