fix: undo StringId -> InternalStringId renaming as it breaks FFI

Author: morrisonlevi

datadog-profiling-ffi/cbindgen.toml

@@ -88,11 +88,8 @@ renaming_overrides_prefixing = true
 "StackTraceId" = "OpaqueStackTraceId"
 
 "GenerationalId_StringId" = "ddog_prof_StringId"
-"GenerationalId_InternalStringId" = "ddog_prof_StringId"
 "Result_GenerationalIdStringId" = "ddog_prof_StringId_Result"
-"Result_GenerationalIdInternalStringId" = "ddog_prof_StringId_Result"
 "MutSlice_GenerationalIdStringId" = "ddog_prof_MutSlice_StringId"
-"MutSlice_GenerationalIdInternalStringId" = "ddog_prof_MutSlice_StringId"
 
 # StringId is an alias of StringOffset, we need both to be `OpaqueStringId`
 # for the current interning API.

datadog-profiling-ffi/src/profiles/interning_api.rs

@@ -6,7 +6,7 @@ use std::num::NonZeroI64;
 use super::datatypes::{profile_ptr_to_inner, Profile};
 use datadog_profiling::{
     api::ManagedStringId,
-    collections::identifiable::InternalStringId,
+    collections::identifiable::StringId,
     internal::{
         self,
         interning_api::{Generation, GenerationalId},
@@ -20,7 +20,7 @@ use ddcommon_ffi::{
 use function_name::named;
 
 // Cbindgen was putting invalid C types on the static, this workaround seems to fix it.
-type CbindgenIsDumbStringId = GenerationalId<InternalStringId>;
+type CbindgenIsDumbStringId = GenerationalId<StringId>;
 
 #[no_mangle]
 #[used]
@@ -43,9 +43,9 @@ pub static ddog_INTERNED_EMPTY_STRING: CbindgenIsDumbStringId =
 #[named]
 pub unsafe extern "C" fn ddog_prof_Profile_intern_function(
     profile: *mut Profile,
-    name: GenerationalId<InternalStringId>,
-    system_name: GenerationalId<InternalStringId>,
-    filename: GenerationalId<InternalStringId>,
+    name: GenerationalId<StringId>,
+    system_name: GenerationalId<StringId>,
+    filename: GenerationalId<StringId>,
 ) -> Result<GenerationalId<InternalFunctionId>> {
     wrap_with_ffi_result!({
         profile_ptr_to_inner(profile)?.intern_function(name, system_name, filename)
@@ -68,14 +68,14 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_function(
 #[named]
 pub unsafe extern "C" fn ddog_prof_Profile_intern_label_num(
     profile: *mut Profile,
-    key: GenerationalId<InternalStringId>,
+    key: GenerationalId<StringId>,
     val: i64,
 ) -> Result<GenerationalId<LabelId>> {
     wrap_with_ffi_result!({
         profile_ptr_to_inner(profile)?.intern_label_num(
             key,
             val,
-            GenerationalId::new_immortal(InternalStringId::ZERO),
+            GenerationalId::new_immortal(StringId::ZERO),
         )
     })
 }
@@ -96,9 +96,9 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_label_num(
 #[named]
 pub unsafe extern "C" fn ddog_prof_Profile_intern_label_num_with_unit(
     profile: *mut Profile,
-    key: GenerationalId<InternalStringId>,
+    key: GenerationalId<StringId>,
     val: i64,
-    unit: GenerationalId<InternalStringId>,
+    unit: GenerationalId<StringId>,
 ) -> Result<GenerationalId<LabelId>> {
     wrap_with_ffi_result!({ profile_ptr_to_inner(profile)?.intern_label_num(key, val, unit) })
 }
@@ -119,8 +119,8 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_label_num_with_unit(
 #[named]
 pub unsafe extern "C" fn ddog_prof_Profile_intern_label_str(
     profile: *mut Profile,
-    key: GenerationalId<InternalStringId>,
-    val: GenerationalId<InternalStringId>,
+    key: GenerationalId<StringId>,
+    val: GenerationalId<StringId>,
 ) -> Result<GenerationalId<LabelId>> {
     wrap_with_ffi_result!({ profile_ptr_to_inner(profile)?.intern_label_str(key, val) })
 }
@@ -214,7 +214,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_location_with_mapping_id(
 pub unsafe extern "C" fn ddog_prof_Profile_intern_managed_string(
     profile: *mut Profile,
     s: ManagedStringId,
-) -> Result<GenerationalId<InternalStringId>> {
+) -> Result<GenerationalId<StringId>> {
     wrap_with_ffi_result!({ profile_ptr_to_inner(profile)?.intern_managed_string(s) })
 }
 
@@ -235,7 +235,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_managed_string(
 pub unsafe extern "C" fn ddog_prof_Profile_intern_managed_strings(
     profile: *mut Profile,
     strings: Slice<ManagedStringId>,
-    mut out: MutSlice<GenerationalId<InternalStringId>>,
+    mut out: MutSlice<GenerationalId<StringId>>,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
         anyhow::ensure!(strings.len() == out.len());
@@ -263,8 +263,8 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_mapping(
     memory_start: u64,
     memory_limit: u64,
     file_offset: u64,
-    filename: GenerationalId<InternalStringId>,
-    build_id: GenerationalId<InternalStringId>,
+    filename: GenerationalId<StringId>,
+    build_id: GenerationalId<StringId>,
 ) -> Result<GenerationalId<InternalMappingId>> {
     wrap_with_ffi_result!({
         profile_ptr_to_inner(profile)?.intern_mapping(
@@ -349,7 +349,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_stacktrace(
 pub unsafe extern "C" fn ddog_prof_Profile_intern_string(
     profile: *mut Profile,
     s: CharSlice,
-) -> Result<GenerationalId<InternalStringId>> {
+) -> Result<GenerationalId<StringId>> {
     wrap_with_ffi_result!({ profile_ptr_to_inner(profile)?.intern_string(s.try_to_utf8()?) })
 }
 
@@ -358,8 +358,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_intern_string(
 /// # Safety
 /// No preconditions
 #[no_mangle]
-pub unsafe extern "C" fn ddog_prof_Profile_interned_empty_string(
-) -> GenerationalId<InternalStringId> {
+pub unsafe extern "C" fn ddog_prof_Profile_interned_empty_string() -> GenerationalId<StringId> {
     internal::Profile::INTERNED_EMPTY_STRING
 }
 
@@ -380,7 +379,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_interned_empty_string(
 pub unsafe extern "C" fn ddog_prof_Profile_intern_strings(
     profile: *mut Profile,
     strings: Slice<CharSlice>,
-    mut out: MutSlice<GenerationalId<InternalStringId>>,
+    mut out: MutSlice<GenerationalId<StringId>>,
 ) -> VoidResult {
     wrap_with_void_ffi_result!({
         anyhow::ensure!(strings.len() == out.len());

datadog-profiling/benches/interning_strings.rs

@@ -7,7 +7,7 @@ use datadog_profiling::collections::string_table::wordpress_test_data::WORDPRESS
 /// This version is the one we used before having datadog-alloc.
 #[allow(unused)]
 mod old_version {
-    use datadog_profiling::collections::identifiable::{FxIndexSet, Id, InternalStringId};
+    use datadog_profiling::collections::identifiable::{FxIndexSet, Id, StringId};
     pub struct StringTable {
         strings: FxIndexSet<Box<str>>,
     }
@@ -19,7 +19,7 @@ mod old_version {
             Self { strings }
         }
 
-        pub fn intern(&mut self, item: &str) -> InternalStringId {
+        pub fn intern(&mut self, item: &str) -> StringId {
             // For performance, delay converting the [&str] to a [String] until
             // after it has been determined to not exist in the set. This avoids
             // temporary allocations.
@@ -31,7 +31,7 @@ mod old_version {
                     index
                 }
             };
-            InternalStringId::from_offset(index)
+            StringId::from_offset(index)
         }
 
         #[inline]

datadog-profiling/src/collections/identifiable/string_id.rs

@@ -4,9 +4,9 @@
 use super::*;
 use datadog_profiling_protobuf::StringOffset;
 
-pub type InternalStringId = StringOffset;
+pub type StringId = StringOffset;
 
-impl Id for InternalStringId {
+impl Id for StringId {
     type RawId = i64;
 
     fn from_offset(inner: usize) -> Self {

datadog-profiling/src/collections/string_storage.rs

@@ -8,13 +8,13 @@ use std::hash::BuildHasherDefault;
 use std::num::NonZeroU32;
 use std::rc::Rc;
 
-use super::identifiable::InternalStringId;
+use super::identifiable::StringId;
 use super::string_table::StringTable;
 
 #[derive(PartialEq, Debug)]
 struct ManagedStringData {
     str: Rc<str>,
-    cached_seq_num: Cell<Option<(InternalCachedProfileId, InternalStringId)>>,
+    cached_seq_num: Cell<Option<(InternalCachedProfileId, StringId)>>,
     usage_count: Cell<u32>,
 }
 
@@ -170,7 +170,7 @@ impl ManagedStringStorage {
         id: NonZeroU32,
         profile_strings: &mut StringTable,
         cached_profile_id: &CachedProfileId,
-    ) -> anyhow::Result<InternalStringId> {
+    ) -> anyhow::Result<StringId> {
         let data = self.get_data(id.into())?;
 
         match data.cached_seq_num.get() {

datadog-profiling/src/collections/string_table/mod.rs

@@ -7,7 +7,7 @@ pub mod wordpress_test_data;
 
 pub use error::*;
 
-use crate::collections::identifiable::InternalStringId;
+use crate::collections::identifiable::StringId;
 use crate::iter::{IntoLendingIterator, LendingIterator};
 use datadog_alloc::{AllocError, Allocator, ChainAllocator, VirtualAllocator};
 use std::alloc::Layout;
@@ -54,7 +54,7 @@ impl<A: Allocator + Clone> ArenaAllocator for ChainAllocator<A> {}
 type Hasher = core::hash::BuildHasherDefault<rustc_hash::FxHasher>;
 type HashSet<K> = indexmap::IndexSet<K, Hasher>;
 
-/// Holds unique strings and provides [InternalStringId]s that correspond to the order
+/// Holds unique strings and provides [StringId]s that correspond to the order
 /// that the strings were inserted.
 pub struct StringTable {
     /// The bytes of each string stored in `strings` are allocated here.
@@ -121,34 +121,33 @@ impl StringTable {
     }
 
     /// Adds the string to the string table if it isn't present already, and
-    /// returns a [InternalStringId] that corresponds to the order that this string
+    /// returns a [StringId] that corresponds to the order that this string
     /// was originally inserted.
     ///
     /// # Panics
     ///
     /// Panics if an allocator fails, or if the number of strings would exceed
     /// [`u32::MAX`].
-    pub fn intern(&mut self, str: &str) -> InternalStringId {
+    pub fn intern(&mut self, str: &str) -> StringId {
         #[allow(clippy::expect_used)]
         self.try_intern(str).expect("failed to intern string")
     }
 
     /// Adds the string to the string table if it isn't present already, and
-    /// returns a [`InternalStringId`] that corresponds to the order that this string
+    /// returns a [`StringId`] that corresponds to the order that this string
     /// was originally inserted.
     ///
     /// Fails if an allocator fails, or if the number of strings would exceed
     /// [`u32::MAX`].
-    pub fn try_intern(&mut self, str: &str) -> Result<InternalStringId, Error> {
+    pub fn try_intern(&mut self, str: &str) -> Result<StringId, Error> {
         let set = &mut self.strings;
         match set.get_index_of(str) {
             // SAFETY: if it already exists, it must fit in the range.
-            Some(offset) => Ok(unsafe { InternalStringId::try_from(offset).unwrap_unchecked() }),
+            Some(offset) => Ok(unsafe { StringId::try_from(offset).unwrap_unchecked() }),
             None => {
                 // No match. Get the current size of the table, which
                 // corresponds to the StringId it will have when inserted.
-                let string_id =
-                    InternalStringId::try_from(set.len()).map_err(|_| Error::StorageFull)?;
+                let string_id = StringId::try_from(set.len()).map_err(|_| Error::StorageFull)?;
 
                 // Make a new string in the arena, and fudge its lifetime
                 // to appease the borrow checker.
@@ -302,11 +301,11 @@ mod tests {
         let mut table = StringTable::new();
         // The empty string should already be present.
         assert_eq!(1, table.len());
-        assert_eq!(InternalStringId::ZERO, table.intern(""));
+        assert_eq!(StringId::ZERO, table.intern(""));
 
         // Intern a string literal to ensure ?Sized works.
         let string = table.intern("datadog");
-        assert_eq!(InternalStringId::from_offset(1), string);
+        assert_eq!(StringId::from_offset(1), string);
         assert_eq!(2, table.len());
     }
 

datadog-profiling/src/internal/endpoints.rs

@@ -4,9 +4,9 @@
 use super::*;
 
 pub struct Endpoints {
-    pub endpoint_label: InternalStringId,
-    pub local_root_span_id_label: InternalStringId,
-    pub mappings: FxIndexMap<u64, InternalStringId>,
+    pub endpoint_label: StringId,
+    pub local_root_span_id_label: StringId,
+    pub mappings: FxIndexMap<u64, StringId>,
     pub stats: ProfiledEndpointsStats,
 }
 

datadog-profiling/src/internal/function.rs

@@ -8,9 +8,9 @@ use super::*;
 ///  - ids for linked objects use 32-bit numbers instead of 64 bit ones.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub struct Function {
-    pub name: InternalStringId,
-    pub system_name: InternalStringId,
-    pub filename: InternalStringId,
+    pub name: StringId,
+    pub system_name: StringId,
+    pub filename: StringId,
 }
 
 impl Item for Function {

datadog-profiling/src/internal/label.rs

@@ -6,16 +6,13 @@ use datadog_profiling_protobuf::{prost_impls, Record, StringOffset};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub enum InternalLabelValue {
-    Str(InternalStringId),
-    Num {
-        num: i64,
-        num_unit: InternalStringId,
-    },
+    Str(StringId),
+    Num { num: i64, num_unit: StringId },
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
 pub struct InternalLabel {
-    key: InternalStringId,
+    key: StringId,
     value: InternalLabelValue,
 }
 
@@ -28,22 +25,22 @@ impl InternalLabel {
         matches!(self.value, InternalLabelValue::Str(_))
     }
 
-    pub fn get_key(&self) -> InternalStringId {
+    pub fn get_key(&self) -> StringId {
         self.key
     }
 
     pub fn get_value(&self) -> &InternalLabelValue {
         &self.value
     }
 
-    pub fn num(key: InternalStringId, num: i64, num_unit: InternalStringId) -> Self {
+    pub fn num(key: StringId, num: i64, num_unit: StringId) -> Self {
         Self {
             key,
             value: InternalLabelValue::Num { num, num_unit },
         }
     }
 
-    pub fn str(key: InternalStringId, v: InternalStringId) -> Self {
+    pub fn str(key: StringId, v: StringId) -> Self {
         Self {
             key,
             value: InternalLabelValue::Str(v),

datadog-profiling/src/internal/mapping.rs

@@ -18,12 +18,12 @@ pub struct Mapping {
     /// The object this entry is loaded from.  This can be a filename on
     /// disk for the main binary and shared libraries, or virtual
     /// abstractions like "[vdso]".
-    pub filename: InternalStringId,
+    pub filename: StringId,
 
     /// A string that uniquely identifies a particular program version
     /// with high probability. E.g., for binaries generated by GNU tools,
     /// it could be the contents of the .note.gnu.build-id field.
-    pub build_id: InternalStringId,
+    pub build_id: StringId,
 }
 
 impl Item for Mapping {