feat(profiling): make dictionary ids comparable (#1570)

# What does this PR do?

Adds comparability and hashability for dictionary-backed `api2` identity types.

Specifically, this PR adds `Eq`/`PartialEq`/`Hash` for:
- `FunctionId2`
- `MappingId2`
- `api2::Location2`

Documents that these comparisons are intended for values produced by the
same `ProfilesDictionary`.

# Motivation

I intend to use this in a future PR to optimize the internals of the Profile.
I'm opening a separate PR to keep PRs smaller and more reviewable.

# Additional Notes

None.

# How to test the change?

1. Check out branch `levi/api2-eq-hash`.
2. Run compile checks:
   - `cargo check -p libdd-profiling`
   - `cargo check -p libdd-profiling --benches`
3. Confirm:
   - `api2::Location2`/`FunctionId2`/`MappingId2` can be used in `Eq`/`Hash`
     contexts for same-dictionary IDs


Co-authored-by: levi.morrison <levi.morrison@datadoghq.com>

Author: morrisonlevi

libdd-profiling/src/api2.rs

@@ -3,7 +3,11 @@
 
 use crate::profiles::datatypes::{FunctionId2, MappingId2, StringId2};
 
-#[derive(Copy, Clone, Debug, Default)]
+/// A location keyed by `MappingId2`/`FunctionId2` handles.
+///
+/// `Eq`/`Hash` comparisons use those handle values, so they are intended for
+/// data that comes from the same `ProfilesDictionary`.
+#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Hash)]
 #[repr(C)]
 pub struct Location2 {
     pub mapping: MappingId2,
@@ -55,3 +59,51 @@ impl<'a> Label<'a> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::profiles::datatypes::{Function2, Mapping2, ProfilesDictionary};
+    use std::collections::hash_map::DefaultHasher;
+    use std::hash::{Hash, Hasher};
+
+    fn hash_of<T: Hash>(value: &T) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        value.hash(&mut hasher);
+        hasher.finish()
+    }
+
+    #[test]
+    fn location2_equality_and_hash_follow_dictionary_handle_identity() {
+        let dict = ProfilesDictionary::try_new().unwrap();
+        let shared = dict.try_insert_str2("shared").unwrap();
+
+        let function = Function2 {
+            name: shared,
+            system_name: shared,
+            file_name: shared,
+        };
+        let mapping = Mapping2 {
+            memory_start: 1,
+            memory_limit: 2,
+            file_offset: 3,
+            filename: shared,
+            build_id: shared,
+        };
+
+        let location_a = Location2 {
+            mapping: dict.try_insert_mapping2(mapping).unwrap(),
+            function: dict.try_insert_function2(function).unwrap(),
+            address: 42,
+            line: 7,
+        };
+        let location_b = Location2 {
+            mapping: dict.try_insert_mapping2(mapping).unwrap(),
+            function: dict.try_insert_function2(function).unwrap(),
+            address: 42,
+            line: 7,
+        };
+        assert_eq!(location_a, location_b);
+        assert_eq!(hash_of(&location_a), hash_of(&location_b));
+    }
+}

libdd-profiling/src/profiles/datatypes/function.rs

@@ -59,7 +59,10 @@ impl From<Function2> for Function {
 /// performed on; it is not generally guaranteed that ids from one dictionary
 /// can be used in another dictionary, even if it happens to work by
 /// implementation detail.
-#[derive(Clone, Copy, Debug)]
+///
+/// `Eq`/`Hash` and other comparisons are based on this handle value, and are
+/// only intended for ids produced by the same `ProfilesDictionary`.
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 #[repr(transparent)]
 pub struct FunctionId2(pub(crate) *mut Function2);
 
@@ -102,6 +105,9 @@ impl From<SetId<Function>> for FunctionId2 {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::profiles::datatypes::ProfilesDictionary;
+    use std::collections::hash_map::DefaultHasher;
+    use std::hash::{Hash, Hasher};
     use std::mem::offset_of;
 
     #[test]
@@ -121,4 +127,36 @@ mod tests {
             offset_of!(Function2, file_name)
         );
     }
+
+    fn hash_of<T: Hash>(value: &T) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        value.hash(&mut hasher);
+        hasher.finish()
+    }
+
+    #[test]
+    fn function_id2_equality_and_hash_are_pointer_identity_based() {
+        let dict = ProfilesDictionary::try_new().unwrap();
+        let name = dict.try_insert_str2("name").unwrap();
+        let system_name = dict.try_insert_str2("system").unwrap();
+        let file_name = dict.try_insert_str2("file").unwrap();
+
+        let function = Function2 {
+            name,
+            system_name,
+            file_name,
+        };
+        let same_a = dict.try_insert_function2(function).unwrap();
+        let same_b = dict.try_insert_function2(function).unwrap();
+        assert_eq!(same_a, same_b);
+        assert_eq!(hash_of(&same_a), hash_of(&same_b));
+
+        let other = Function2 {
+            name: dict.try_insert_str2("other_name").unwrap(),
+            system_name,
+            file_name,
+        };
+        let different = dict.try_insert_function2(other).unwrap();
+        assert_ne!(same_a, different);
+    }
 }

libdd-profiling/src/profiles/datatypes/mapping.rs

@@ -67,7 +67,10 @@ impl From<Mapping> for Mapping2 {
 /// performed on; it is not generally guaranteed that ids from one dictionary
 /// can be used in another dictionary, even if it happens to work by
 /// implementation detail.
-#[derive(Clone, Copy, Debug)]
+///
+/// `Eq`/`Hash` and other comparisons are based on this handle value, and are
+/// only intended for ids produced by the same `ProfilesDictionary`.
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 #[repr(transparent)]
 pub struct MappingId2(pub(crate) *mut Mapping2);
 
@@ -110,6 +113,9 @@ impl From<SetId<Mapping>> for MappingId2 {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::profiles::datatypes::ProfilesDictionary;
+    use std::collections::hash_map::DefaultHasher;
+    use std::hash::{Hash, Hasher};
     use std::mem::offset_of;
 
     #[test]
@@ -140,4 +146,37 @@ mod tests {
             offset_of!(Mapping2, build_id)
         );
     }
+
+    fn hash_of<T: Hash>(value: &T) -> u64 {
+        let mut hasher = DefaultHasher::new();
+        value.hash(&mut hasher);
+        hasher.finish()
+    }
+
+    #[test]
+    fn mapping_id2_equality_and_hash_are_pointer_identity_based() {
+        let dict = ProfilesDictionary::try_new().unwrap();
+        let filename = dict.try_insert_str2("filename").unwrap();
+        let build_id = dict.try_insert_str2("build").unwrap();
+
+        let mapping = Mapping2 {
+            memory_start: 100,
+            memory_limit: 200,
+            file_offset: 10,
+            filename,
+            build_id,
+        };
+        let same_a = dict.try_insert_mapping2(mapping).unwrap();
+        let same_b = dict.try_insert_mapping2(mapping).unwrap();
+        assert_eq!(same_a, same_b);
+        assert_eq!(hash_of(&same_a), hash_of(&same_b));
+
+        let different = dict
+            .try_insert_mapping2(Mapping2 {
+                memory_start: 101,
+                ..mapping
+            })
+            .unwrap();
+        assert_ne!(same_a, different);
+    }
 }