From 9a6a9451ba4d8c893d1aa47155298491a761ee9f Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 27 Sep 2024 16:25:50 -0400 Subject: [PATCH] refactor: zero-copy deserialization when possible Found this during PR review. We could leverage `#[serde(borrow)]` for zero-copy deserialization for messages from the compiler. We can't use `&str` fields because they may contain escape sequences in the future, which fails the deserialization. See https://github.com/serde-rs/json/issues/742 --- .../core/compiler/job_queue/job_state.rs | 4 +- src/cargo/core/compiler/mod.rs | 37 ++++++++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/compiler/job_queue/job_state.rs b/src/cargo/core/compiler/job_queue/job_state.rs index 12c04258d..b3b2e8960 100644 --- a/src/cargo/core/compiler/job_queue/job_state.rs +++ b/src/cargo/core/compiler/job_queue/job_state.rs @@ -103,7 +103,7 @@ impl<'a, 'gctx> JobState<'a, 'gctx> { } /// See [`Message::Diagnostic`] and [`Message::WarningCount`]. - pub fn emit_diag(&self, level: String, diag: String, fixable: bool) -> CargoResult<()> { + pub fn emit_diag(&self, level: &str, diag: String, fixable: bool) -> CargoResult<()> { if let Some(dedupe) = self.output { let emitted = dedupe.emit_diag(&diag)?; if level == "warning" { @@ -116,7 +116,7 @@ impl<'a, 'gctx> JobState<'a, 'gctx> { } else { self.messages.push_bounded(Message::Diagnostic { id: self.id, - level, + level: level.to_string(), diag, fixable, }); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 4592fdfa3..34cf679de 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -54,6 +54,7 @@ mod unit; pub mod unit_dependencies; pub mod unit_graph; +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::env; use std::ffi::{OsStr, OsString}; @@ -1756,10 +1757,15 @@ fn on_stderr_line_inner( .. } => { #[derive(serde::Deserialize)] - struct CompilerMessage { + struct CompilerMessage<'a> { + // `rendered` contains escape sequences, which can't be + // zero-copy deserialized by serde_json. + // See https://github.com/serde-rs/json/issues/742 rendered: String, - message: String, - level: String, + #[serde(borrow)] + message: Cow<'a, str>, + #[serde(borrow)] + level: Cow<'a, str>, children: Vec, } @@ -1782,7 +1788,8 @@ fn on_stderr_line_inner( suggestion_applicability: Option, } - if let Ok(mut msg) = serde_json::from_str::(compiler_message.get()) { + if let Ok(mut msg) = serde_json::from_str::>(compiler_message.get()) + { if msg.message.starts_with("aborting due to") || msg.message.ends_with("warning emitted") || msg.message.ends_with("warnings emitted") @@ -1808,7 +1815,7 @@ fn on_stderr_line_inner( }) .any(|b| b); count_diagnostic(&msg.level, options); - state.emit_diag(msg.level, rendered, machine_applicable)?; + state.emit_diag(&msg.level, rendered, machine_applicable)?; } return Ok(true); } @@ -1819,12 +1826,14 @@ fn on_stderr_line_inner( // cached replay to enable/disable colors without re-invoking rustc. MessageFormat::Json { ansi: false, .. } => { #[derive(serde::Deserialize, serde::Serialize)] - struct CompilerMessage { + struct CompilerMessage<'a> { rendered: String, - #[serde(flatten)] - other: std::collections::BTreeMap, + #[serde(flatten, borrow)] + other: std::collections::BTreeMap, serde_json::Value>, } - if let Ok(mut error) = serde_json::from_str::(compiler_message.get()) { + if let Ok(mut error) = + serde_json::from_str::>(compiler_message.get()) + { error.rendered = anstream::adapter::strip_str(&error.rendered).to_string(); let new_line = serde_json::to_string(&error)?; let new_msg: Box = serde_json::from_str(&new_line)?; @@ -1866,12 +1875,14 @@ fn on_stderr_line_inner( } #[derive(serde::Deserialize)] - struct CompilerMessage { - message: String, - level: String, + struct CompilerMessage<'a> { + #[serde(borrow)] + message: Cow<'a, str>, + #[serde(borrow)] + level: Cow<'a, str>, } - if let Ok(msg) = serde_json::from_str::(compiler_message.get()) { + if let Ok(msg) = serde_json::from_str::>(compiler_message.get()) { if msg.message.starts_with("aborting due to") || msg.message.ends_with("warning emitted") || msg.message.ends_with("warnings emitted")