Various performance-related improvements (#675)

This fixes various allocations and single-threadedness that I noticed
while profiling https://github.com/mstange/reliost which uses wholesym's
query_api method to process the JSON symbolication requests.

Author: mstange

Cargo.lock

@@ -1,8 +1,9 @@
+use std::borrow::Cow;
+
 use samply_symbols::SourceFilePath;
 
-pub fn to_api_file_path(file_path: &SourceFilePath) -> String {
-    match file_path.mapped_path() {
-        Some(mapped_path) => mapped_path.to_special_path_str(),
-        None => file_path.raw_path().to_owned(),
-    }
+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

@@ -5,21 +5,17 @@ use samply_symbols::{
     object, CodeByteReadingError, CodeId, FileAndPathHelper, FileAndPathHelperError, LibraryInfo,
     LookupAddress, SymbolManager,
 };
-use serde_json::json;
 use yaxpeax_arch::{Arch, DecodeError, LengthedInstruction, Reader, U8Reader};
 use yaxpeax_x86::amd64::{Opcode, Operand};
 
 use self::response_json::Response;
 use crate::asm::response_json::DecodedInstruction;
 
-mod request_json;
-mod response_json;
+pub mod request_json;
+pub mod response_json;
 
 #[derive(thiserror::Error, Debug)]
-enum AsmError {
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
-
+pub enum AsmError {
     #[error("An error occurred when loading the binary: {0}")]
     LoadBinaryError(#[from] samply_symbols::Error),
 
@@ -49,20 +45,15 @@ impl<'a, H: FileAndPathHelper> AsmApi<'a, H> {
         Self { symbol_manager }
     }
 
-    pub async fn query_api_json(&self, request_json: &str) -> String {
-        match self.query_api_fallible_json(request_json).await {
-            Ok(response_json) => response_json,
-            Err(err) => json!({ "error": err.to_string() }).to_string(),
-        }
-    }
-
-    async fn query_api_fallible_json(&self, request_json: &str) -> Result<String, AsmError> {
+    pub async fn query_api_json(
+        &self,
+        request_json: &str,
+    ) -> Result<response_json::Response, crate::Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
-        let response = self.query_api(&request).await?;
-        Ok(serde_json::to_string(&response)?)
+        Ok(self.query_api(&request).await?)
     }
 
-    async fn query_api(
+    pub async fn query_api(
         &self,
         request: &request_json::Request,
     ) -> Result<response_json::Response, AsmError> {

samply-api/src/asm/mod.rs

@@ -1,17 +1,44 @@
+use serde::Serialize;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum Error {
+    #[error("Unrecognized URL: {0}")]
+    UnrecognizedUrl(String),
+
+    #[error("Couldn't parse request: {0}")]
+    ParseRequestErrorSerde(#[from] serde_json::error::Error),
+
+    #[error("Malformed request JSON: {0}")]
+    ParseRequestErrorContents(&'static str),
+
     #[error("{0}")]
     Symbols(
         #[from]
         #[source]
         samply_symbols::Error,
     ),
 
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
+    #[error("{0}")]
+    Source(
+        #[from]
+        #[source]
+        super::source::SourceError,
+    ),
 
-    #[error("Malformed request JSON: {0}")]
-    ParseRequestErrorContents(&'static str),
+    #[error("{0}")]
+    Asm(
+        #[from]
+        #[source]
+        super::asm::AsmError,
+    ),
+}
+
+impl Serialize for Error {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.to_string().serialize(serializer)
+    }
 }

samply-api/src/error.rs

@@ -6,20 +6,6 @@ where
     serializer.collect_str(&format_args!("{field:#x}"))
 }
 
-pub fn as_optional_hex_string<S, T>(
-    field: &Option<T>,
-    serializer: S,
-) -> std::result::Result<S::Ok, S::Error>
-where
-    S: serde::Serializer,
-    T: std::fmt::LowerHex,
-{
-    match field {
-        Some(field) => serializer.collect_str(&format_args!("{field:#x}")),
-        None => serializer.serialize_none(),
-    }
-}
-
 pub fn from_prefixed_hex_str<'de, D>(deserializer: D) -> Result<u32, D::Error>
 where
     D: serde::Deserializer<'de>,

samply-api/src/hex.rs

@@ -28,7 +28,7 @@
 //!     let symbol_manager = SymbolManager::with_helper(helper);
 //!     let api = samply_api::Api::new(&symbol_manager);
 //!
-//!     api.query_api(
+//!     let api_result = api.query_api(
 //!         "/symbolicate/v5",
 //!         r#"{
 //!             "memoryMap": [
@@ -46,7 +46,8 @@
 //!               ]
 //!             ]
 //!           }"#,
-//!     ).await
+//!     ).await;
+//!     serde_json::to_string(&api_result).unwrap()
 //! }
 //!
 //! struct ExampleHelper {
@@ -147,7 +148,7 @@ use debugid::DebugId;
 pub use samply_symbols;
 pub use samply_symbols::debugid;
 use samply_symbols::{FileAndPathHelper, SymbolManager};
-use serde_json::json;
+use serde::Serialize;
 use source::SourceApi;
 use symbolicate::SymbolicateApi;
 
@@ -158,6 +159,8 @@ mod hex;
 mod source;
 mod symbolicate;
 
+pub use error::Error;
+
 pub(crate) fn to_debug_id(breakpad_id: &str) -> Result<DebugId, samply_symbols::Error> {
     // Only accept breakpad IDs with the right syntax, and which aren't all-zeros.
     match DebugId::from_breakpad(breakpad_id) {
@@ -168,12 +171,57 @@ pub(crate) fn to_debug_id(breakpad_id: &str) -> Result<DebugId, samply_symbols::
     }
 }
 
+pub enum QueryApiJsonResult<H: FileAndPathHelper> {
+    SymbolicateResponse(symbolicate::response_json::Response<H>),
+    SourceResponse(source::response_json::Response),
+    AsmResponse(asm::response_json::Response),
+    Err(Error),
+}
+
+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<H: FileAndPathHelper> From<source::response_json::Response> for QueryApiJsonResult<H> {
+    fn from(value: source::response_json::Response) -> Self {
+        QueryApiJsonResult::SourceResponse(value)
+    }
+}
+
+impl<H: FileAndPathHelper> From<asm::response_json::Response> for QueryApiJsonResult<H> {
+    fn from(value: asm::response_json::Response) -> Self {
+        QueryApiJsonResult::AsmResponse(value)
+    }
+}
+
+impl<H: FileAndPathHelper> From<Error> for QueryApiJsonResult<H> {
+    fn from(value: Error) -> Self {
+        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),
+        }
+    }
+}
+
 #[derive(Clone, Copy)]
 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 }
@@ -194,18 +242,34 @@ 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) -> String {
+    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())
+    }
+    pub async fn query_api_fallible(
+        self,
+        request_url: &str,
+        request_json_data: &str,
+    ) -> Result<QueryApiJsonResult<H>, Error> {
         if request_url == "/symbolicate/v5" {
             let symbolicate_api = SymbolicateApi::new(self.symbol_manager);
-            symbolicate_api.query_api_json(request_json_data).await
+            Ok(symbolicate_api
+                .query_api_json(request_json_data)
+                .await?
+                .into())
         } else if request_url == "/source/v1" {
             let source_api = SourceApi::new(self.symbol_manager);
-            source_api.query_api_json(request_json_data).await
+            Ok(source_api.query_api_json(request_json_data).await?.into())
         } else if request_url == "/asm/v1" {
             let asm_api = AsmApi::new(self.symbol_manager);
-            asm_api.query_api_json(request_json_data).await
+            Ok(asm_api.query_api_json(request_json_data).await?.into())
         } else {
-            json!({ "error": format!("Unrecognized URL {request_url}") }).to_string()
+            Err(Error::UnrecognizedUrl(request_url.into()))
         }
     }
 }

samply-api/src/lib.rs

@@ -1,19 +1,15 @@
 use samply_symbols::{
     FileAndPathHelper, FileAndPathHelperError, LibraryInfo, LookupAddress, SymbolManager,
 };
-use serde_json::json;
 
 use crate::api_file_path::to_api_file_path;
-use crate::to_debug_id;
+use crate::{to_debug_id, Error};
 
-mod request_json;
-mod response_json;
+pub mod request_json;
+pub mod response_json;
 
 #[derive(thiserror::Error, Debug)]
-enum SourceError {
-    #[error("Couldn't parse request: {0}")]
-    ParseRequestErrorSerde(#[from] serde_json::error::Error),
-
+pub enum SourceError {
     #[error("Could not obtain symbols for the requested library: {0}")]
     NoSymbols(#[from] samply_symbols::Error),
 
@@ -37,20 +33,15 @@ impl<'a, H: FileAndPathHelper> SourceApi<'a, H> {
         Self { symbol_manager }
     }
 
-    pub async fn query_api_json(&self, request_json: &str) -> String {
-        match self.query_api_fallible_json(request_json).await {
-            Ok(response_json) => response_json,
-            Err(err) => json!({ "error": err.to_string() }).to_string(),
-        }
-    }
-
-    async fn query_api_fallible_json(&self, request_json: &str) -> Result<String, SourceError> {
+    pub async fn query_api_json(
+        &self,
+        request_json: &str,
+    ) -> Result<response_json::Response, Error> {
         let request: request_json::Request = serde_json::from_str(request_json)?;
-        let response = self.query_api(&request).await?;
-        Ok(serde_json::to_string(&response)?)
+        Ok(self.query_api(&request).await?)
     }
 
-    async fn query_api(
+    pub async fn query_api(
         &self,
         request: &request_json::Request,
     ) -> Result<response_json::Response, SourceError> {
@@ -82,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,