From 821d8eaec34a0a04d86dadbbb56cd26e0214b455 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 7 Nov 2020 12:28:31 +0100 Subject: [PATCH 1/3] Avoid constructing an anyhow::Error when not necessary anyhow::Error always captures a backtrace when created, which is expensive --- src/cargo/core/compiler/job_queue.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a3e630372ae..9ebcf96d124 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -53,7 +53,6 @@ use std::cell::Cell; use std::collections::{BTreeMap, HashMap, HashSet}; use std::io; use std::marker; -use std::mem; use std::sync::Arc; use std::time::Duration; @@ -838,9 +837,10 @@ impl<'cfg> DrainState<'cfg> { let mut sender = FinishOnDrop { messages: &messages, id, - result: Err(format_err!("worker panicked")), + ok: false, }; - sender.result = job.run(&state); + let result = job.run(&state); + sender.ok = true; // If the `rmeta_required` wasn't consumed but it was set // previously, then we either have: @@ -854,10 +854,12 @@ impl<'cfg> DrainState<'cfg> { // we'll just naturally abort the compilation operation but for 1 // we need to make sure that the metadata is flagged as produced so // send a synthetic message here. - if state.rmeta_required.get() && sender.result.is_ok() { + if state.rmeta_required.get() && result.is_ok() { messages.push(Message::Finish(id, Artifact::Metadata, Ok(()))); } + messages.push(Message::Finish(id, Artifact::All, result)); + // Use a helper struct with a `Drop` implementation to guarantee // that a `Finish` message is sent even if our job panics. We // shouldn't panic unless there's a bug in Cargo, so we just need @@ -865,14 +867,18 @@ impl<'cfg> DrainState<'cfg> { struct FinishOnDrop<'a> { messages: &'a Queue, id: JobId, - result: CargoResult<()>, + ok: bool, } impl Drop for FinishOnDrop<'_> { fn drop(&mut self) { - let msg = mem::replace(&mut self.result, Ok(())); - self.messages - .push(Message::Finish(self.id, Artifact::All, msg)); + if !self.ok { + self.messages.push(Message::Finish( + self.id, + Artifact::All, + Err(format_err!("worker panicked")), + )); + } } } }; From 5af00d0e61bd5ca6fbdf378c6a11be0ac7dd38e6 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 9 Nov 2020 17:21:18 +0100 Subject: [PATCH 2/3] Always send message through FinishOnDrop --- src/cargo/core/compiler/job_queue.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 9ebcf96d124..791c8fd170c 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -837,10 +837,9 @@ impl<'cfg> DrainState<'cfg> { let mut sender = FinishOnDrop { messages: &messages, id, - ok: false, + result: None, }; - let result = job.run(&state); - sender.ok = true; + sender.result = Some(job.run(&state)); // If the `rmeta_required` wasn't consumed but it was set // previously, then we either have: @@ -854,12 +853,10 @@ impl<'cfg> DrainState<'cfg> { // we'll just naturally abort the compilation operation but for 1 // we need to make sure that the metadata is flagged as produced so // send a synthetic message here. - if state.rmeta_required.get() && result.is_ok() { + if state.rmeta_required.get() && sender.result.as_ref().unwrap().is_ok() { messages.push(Message::Finish(id, Artifact::Metadata, Ok(()))); } - messages.push(Message::Finish(id, Artifact::All, result)); - // Use a helper struct with a `Drop` implementation to guarantee // that a `Finish` message is sent even if our job panics. We // shouldn't panic unless there's a bug in Cargo, so we just need @@ -867,12 +864,15 @@ impl<'cfg> DrainState<'cfg> { struct FinishOnDrop<'a> { messages: &'a Queue, id: JobId, - ok: bool, + result: Option>, } impl Drop for FinishOnDrop<'_> { fn drop(&mut self) { - if !self.ok { + if let Some(result) = self.result.take() { + self.messages + .push(Message::Finish(self.id, Artifact::All, result)); + } else { self.messages.push(Message::Finish( self.id, Artifact::All, From e643df0985528e5fd0b826db0b7bc07900716fa9 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 9 Nov 2020 17:38:31 +0100 Subject: [PATCH 3/3] Simplify using unwrap_or_else --- src/cargo/core/compiler/job_queue.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 791c8fd170c..b8b22ae5a2f 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -869,16 +869,12 @@ impl<'cfg> DrainState<'cfg> { impl Drop for FinishOnDrop<'_> { fn drop(&mut self) { - if let Some(result) = self.result.take() { - self.messages - .push(Message::Finish(self.id, Artifact::All, result)); - } else { - self.messages.push(Message::Finish( - self.id, - Artifact::All, - Err(format_err!("worker panicked")), - )); - } + let result = self + .result + .take() + .unwrap_or_else(|| Err(format_err!("worker panicked"))); + self.messages + .push(Message::Finish(self.id, Artifact::All, result)); } } };