Add SourceFilePathHandle.

The address result now contains SourceFilePathHandles instead of
the actual SourceFilePath.
The caller then has to resolve the handle to an actual SourceFilePath
in a separate step.

This achieves the following goals:
- No string copies are allocated for the address result.
- The address result is still "owned", i.e. it can be passed
  around without having a lifetime dependency on the SymbolMap.
- The handle can be used as a hash map key, hashing the key is
  fast and independent of the string length.
- It's easier for callers to do per-path work only once.
- Making path strings that live longer than the SymbolMap is now
  an explicit operation. (e.g. this approach doesn't use Arc<str>)
- Resolving paths often doesn't have to allocate strings either
  because the returned SourceFilePath can have a lifetime dependency
  on the SymbolMap, which means it can reference the file data whose
  mmap handle is owned by the SymbolMap.

In practice this massively reduces the number of allocations + frees
happening when handling the symbolication JSON API request.

Author: mstange

Cargo.lock

@@ -2,7 +2,7 @@ use std::borrow::Cow;
 
 use samply_symbols::SourceFilePath;
 
-pub fn to_api_file_path(file_path: &SourceFilePath) -> Cow<'_, str> {
+pub fn to_api_file_path<'a>(file_path: &'a SourceFilePath<'_>) -> Cow<'a, str> {
     file_path
         .special_path_str()
         .unwrap_or_else(|| file_path.raw_path().into())

samply-api/src/api_file_path.rs

@@ -148,6 +148,7 @@ use debugid::DebugId;
 pub use samply_symbols;
 pub use samply_symbols::debugid;
 use samply_symbols::{FileAndPathHelper, SymbolManager};
+use serde::Serialize;
 use source::SourceApi;
 use symbolicate::SymbolicateApi;
 
@@ -170,39 +171,48 @@ pub(crate) fn to_debug_id(breakpad_id: &str) -> Result<DebugId, samply_symbols::
     }
 }
 
-#[derive(serde_derive::Serialize)]
-pub struct QueryApiJsonResult(QueryApiJsonResultInner);
-
-#[derive(serde_derive::Serialize)]
-#[serde(untagged)]
-enum QueryApiJsonResultInner {
-    SymbolicateResponse(symbolicate::response_json::Response),
+pub enum QueryApiJsonResult<H: FileAndPathHelper> {
+    SymbolicateResponse(symbolicate::response_json::Response<H>),
     SourceResponse(source::response_json::Response),
     AsmResponse(asm::response_json::Response),
     Err(Error),
 }
 
-impl From<symbolicate::response_json::Response> for QueryApiJsonResult {
-    fn from(value: symbolicate::response_json::Response) -> Self {
-        Self(QueryApiJsonResultInner::SymbolicateResponse(value))
+impl<H: FileAndPathHelper> From<symbolicate::response_json::Response<H>> for QueryApiJsonResult<H> {
+    fn from(value: symbolicate::response_json::Response<H>) -> Self {
+        QueryApiJsonResult::SymbolicateResponse(value)
     }
 }
 
-impl From<source::response_json::Response> for QueryApiJsonResult {
+impl<H: FileAndPathHelper> From<source::response_json::Response> for QueryApiJsonResult<H> {
     fn from(value: source::response_json::Response) -> Self {
-        Self(QueryApiJsonResultInner::SourceResponse(value))
+        QueryApiJsonResult::SourceResponse(value)
     }
 }
 
-impl From<asm::response_json::Response> for QueryApiJsonResult {
+impl<H: FileAndPathHelper> From<asm::response_json::Response> for QueryApiJsonResult<H> {
     fn from(value: asm::response_json::Response) -> Self {
-        Self(QueryApiJsonResultInner::AsmResponse(value))
+        QueryApiJsonResult::AsmResponse(value)
     }
 }
 
-impl From<Error> for QueryApiJsonResult {
+impl<H: FileAndPathHelper> From<Error> for QueryApiJsonResult<H> {
     fn from(value: Error) -> Self {
-        Self(QueryApiJsonResultInner::Err(value))
+        QueryApiJsonResult::Err(value)
+    }
+}
+
+impl<H: FileAndPathHelper> Serialize for QueryApiJsonResult<H> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        match self {
+            QueryApiJsonResult::SymbolicateResponse(response) => response.serialize(serializer),
+            QueryApiJsonResult::SourceResponse(response) => response.serialize(serializer),
+            QueryApiJsonResult::AsmResponse(response) => response.serialize(serializer),
+            QueryApiJsonResult::Err(error) => error.serialize(serializer),
+        }
     }
 }
 
@@ -211,7 +221,7 @@ pub struct Api<'a, H: FileAndPathHelper> {
     symbol_manager: &'a SymbolManager<H>,
 }
 
-impl<'a, H: FileAndPathHelper> Api<'a, H> {
+impl<'a, H: FileAndPathHelper + 'static> Api<'a, H> {
     /// Create a [`Api`] instance which uses the provided [`SymbolManager`].
     pub fn new(symbol_manager: &'a SymbolManager<H>) -> Self {
         Self { symbol_manager }
@@ -232,7 +242,11 @@ impl<'a, H: FileAndPathHelper> Api<'a, H> {
     ///    symbol information for that address.
     ///  - `/asm/v1`: Experimental API. Symbolicates an address and lets you read one of the files in the
     ///    symbol information for that address.
-    pub async fn query_api(self, request_url: &str, request_json_data: &str) -> QueryApiJsonResult {
+    pub async fn query_api(
+        self,
+        request_url: &str,
+        request_json_data: &str,
+    ) -> QueryApiJsonResult<H> {
         self.query_api_fallible(request_url, request_json_data)
             .await
             .unwrap_or_else(|e| e.into())
@@ -241,7 +255,7 @@ impl<'a, H: FileAndPathHelper> Api<'a, H> {
         self,
         request_url: &str,
         request_json_data: &str,
-    ) -> Result<QueryApiJsonResult, Error> {
+    ) -> Result<QueryApiJsonResult<H>, Error> {
         if request_url == "/symbolicate/v5" {
             let symbolicate_api = SymbolicateApi::new(self.symbol_manager);
             Ok(symbolicate_api

samply-api/src/lib.rs

@@ -73,6 +73,7 @@ impl<'a, H: FileAndPathHelper> SourceApi<'a, H> {
         let source_file_path = frames
             .into_iter()
             .filter_map(|frame| frame.file_path)
+            .map(|path| symbol_map.resolve_source_file_path(path))
             .find(|file_path| to_api_file_path(file_path) == *requested_file)
             .ok_or(SourceError::InvalidPath)?;
 

samply-api/src/source/mod.rs

@@ -1,6 +1,16 @@
 use std::collections::BTreeMap;
 
-use samply_symbols::FrameDebugInfo;
+use samply_symbols::{FrameDebugInfo, SourceFilePath, SourceFilePathHandle};
+
+pub trait PathResolver {
+    fn resolve_source_file_path(&self, handle: SourceFilePathHandle) -> SourceFilePath<'_>;
+}
+
+impl PathResolver for () {
+    fn resolve_source_file_path(&self, _handle: SourceFilePathHandle) -> SourceFilePath<'_> {
+        unreachable!()
+    }
+}
 
 pub struct AddressResult {
     pub symbol_address: u32,

samply-api/src/symbolicate/looked_up_addresses.rs

@@ -1,10 +1,14 @@
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use samply_symbols::{
-    FileAndPathHelper, FramesLookupResult, LibraryInfo, LookupAddress, SymbolManager,
+    FileAndPathHelper, FramesLookupResult, LibraryInfo, LookupAddress, SourceFilePath,
+    SourceFilePathHandle, SymbolManager, SymbolMap,
 };
 
 use crate::error::Error;
+use crate::symbolicate::looked_up_addresses::PathResolver;
+use crate::symbolicate::response_json::{PerLibResult, Response};
 use crate::to_debug_id;
 
 pub mod looked_up_addresses;
@@ -14,11 +18,17 @@ pub mod response_json;
 use looked_up_addresses::LookedUpAddresses;
 use request_json::Lib;
 
+impl<H: FileAndPathHelper> PathResolver for SymbolMap<H> {
+    fn resolve_source_file_path(&self, handle: SourceFilePathHandle) -> SourceFilePath<'_> {
+        self.resolve_source_file_path(handle)
+    }
+}
+
 pub struct SymbolicateApi<'a, H: FileAndPathHelper> {
     symbol_manager: &'a SymbolManager<H>,
 }
 
-impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
+impl<'a, H: FileAndPathHelper + 'static> SymbolicateApi<'a, H> {
     /// Create a [`SymbolicateApi`] instance which uses the provided [`SymbolManager`].
     pub fn new(symbol_manager: &'a SymbolManager<H>) -> Self {
         Self { symbol_manager }
@@ -27,26 +37,29 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
     pub async fn query_api_json(
         &self,
         request_json: &str,
-    ) -> Result<response_json::Response, Error> {
+    ) -> Result<response_json::Response<H>, Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
         self.query_api(request).await
     }
 
     pub async fn query_api(
         &self,
         request: request_json::Request,
-    ) -> Result<response_json::Response, Error> {
+    ) -> Result<response_json::Response<H>, Error> {
         let requested_addresses = gather_requested_addresses(&request)?;
-        let symbolicated_addresses = self
+        let per_lib_results = self
             .symbolicate_requested_addresses(requested_addresses)
             .await;
-        Ok(create_response(request, symbolicated_addresses))
+        Ok(Response {
+            request,
+            per_lib_results,
+        })
     }
 
     async fn symbolicate_requested_addresses(
         &self,
         requested_addresses: HashMap<Lib, Vec<u32>>,
-    ) -> HashMap<Lib, Result<LookedUpAddresses, samply_symbols::Error>> {
+    ) -> HashMap<Lib, Result<PerLibResult<H>, samply_symbols::Error>> {
         let mut symbolicated_addresses = HashMap::new();
         for (lib, addresses) in requested_addresses.into_iter() {
             let address_results = self
@@ -61,15 +74,14 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
         &self,
         lib: &Lib,
         mut addresses: Vec<u32>,
-    ) -> Result<LookedUpAddresses, samply_symbols::Error> {
+    ) -> Result<PerLibResult<H>, samply_symbols::Error> {
         // Sort the addresses before the lookup, to have a higher chance of hitting
         // the same external file for subsequent addresses.
         addresses.sort_unstable();
         addresses.dedup();
 
         let debug_id = to_debug_id(&lib.breakpad_id)?;
 
-        let mut symbolication_result = LookedUpAddresses::for_addresses(&addresses);
         let mut external_addresses = Vec::new();
 
         // Do the synchronous work first, and accumulate external_addresses which need
@@ -82,6 +94,7 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
             ..Default::default()
         };
         let symbol_map = self.symbol_manager.load_symbol_map(&info).await?;
+        let mut symbolication_result = LookedUpAddresses::for_addresses(&addresses);
 
         symbolication_result.set_total_symbol_count(symbol_map.symbol_count() as u32);
 
@@ -117,7 +130,12 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
             }
         }
 
-        Ok(symbolication_result)
+        let outcome = PerLibResult {
+            address_results: symbolication_result,
+            symbol_map: Arc::new(symbol_map),
+        };
+
+        Ok(outcome)
     }
 }
 
@@ -147,15 +165,3 @@ fn gather_requested_addresses(
     }
     Ok(requested_addresses)
 }
-
-fn create_response(
-    request: request_json::Request,
-    symbolicated_addresses: HashMap<Lib, Result<LookedUpAddresses, samply_symbols::Error>>,
-) -> response_json::Response {
-    use response_json::{Response, Results};
-
-    Response(Results {
-        request,
-        symbolicated_addresses,
-    })
-}