From 2c64e54bd45547a90bb2038f53ff681c624e9977 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 19 Jan 2026 18:46:38 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20Bolt:=20optimize=20command=20execut?= =?UTF-8?q?ion=20by=20reducing=20redundant=20clones?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit optimizes the bootstrap build system by: 1. Changing `CommandOutput` to use `Arc<[u8]>` instead of `Vec` for captured stdout/stderr. This makes `CommandOutput` cloning O(1) and avoids redundant data copying when retrieving command results from the cache. 2. Avoiding redundant `fingerprint()` calculations in `DeferredCommand::finish_process` by passing the already existing fingerprint from the caller. These changes reduce memory allocations and data copying during the build process, especially for commands that are frequently cached or have large outputs. --- .jules/bolt.md | 3 +++ .jules/sentinel.md | 4 +++ src/bootstrap/src/utils/exec.rs | 44 ++++++++++++++++++++++----------- 3 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 .jules/bolt.md create mode 100644 .jules/sentinel.md diff --git a/.jules/bolt.md b/.jules/bolt.md new file mode 100644 index 0000000000000..d3420067bbfc2 --- /dev/null +++ b/.jules/bolt.md @@ -0,0 +1,3 @@ +## 2025-05-15 - Redundant data cloning in command execution +**Learning:** In the command execution module, cache hits were causing redundant clones of the entire captured stdout/stderr. By using `Arc<[u8]>` instead of `Vec` in `CommandOutput`, we can avoid copying large outputs when retrieving from the cache or when cloning the output object. Additionally, `fingerprint()` was being called redundantly on the failure path, which involves cloning all arguments and environment variables. +**Action:** Use `Arc` for shared data in performance-critical structures that are frequently cloned. Avoid redundant calculations of complex objects like command fingerprints by passing them down the call stack. diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000000000..0376421574f1d --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-05-15 - No security issues found +**Vulnerability:** None identified in this session. +**Learning:** The bootstrap build system is relatively self-contained and doesn't handle sensitive user data or external network requests in a way that exposed common vulnerabilities. +**Prevention:** Continue to monitor for insecure use of `Command` and potential path traversal in build steps. diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 61b8b26dceaf4..a261726041932 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -445,8 +445,8 @@ pub fn command>(program: S) -> BootstrapCommand { #[derive(Clone, PartialEq)] pub struct CommandOutput { status: CommandStatus, - stdout: Option>, - stderr: Option>, + stdout: Option>, + stderr: Option>, } impl CommandOutput { @@ -456,11 +456,11 @@ impl CommandOutput { status: CommandStatus::DidNotStartOrFinish, stdout: match stdout { OutputMode::Print => None, - OutputMode::Capture => Some(vec![]), + OutputMode::Capture => Some(Arc::from(vec![])), }, stderr: match stderr { OutputMode::Print => None, - OutputMode::Capture => Some(vec![]), + OutputMode::Capture => Some(Arc::from(vec![])), }, } } @@ -471,11 +471,11 @@ impl CommandOutput { status: CommandStatus::Finished(output.status), stdout: match stdout { OutputMode::Print => None, - OutputMode::Capture => Some(output.stdout), + OutputMode::Capture => Some(Arc::from(output.stdout)), }, stderr: match stderr { OutputMode::Print => None, - OutputMode::Capture => Some(output.stderr), + OutputMode::Capture => Some(Arc::from(output.stderr)), }, } } @@ -503,14 +503,17 @@ impl CommandOutput { #[must_use] pub fn stdout(&self) -> String { String::from_utf8( - self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"), + self.stdout + .as_ref() + .expect("Accessing stdout of a command that did not capture stdout") + .to_vec(), ) .expect("Cannot parse process stdout as UTF-8") } #[must_use] pub fn stdout_if_present(&self) -> Option { - self.stdout.as_ref().and_then(|s| String::from_utf8(s.clone()).ok()) + self.stdout.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok()) } #[must_use] @@ -521,14 +524,17 @@ impl CommandOutput { #[must_use] pub fn stderr(&self) -> String { String::from_utf8( - self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"), + self.stderr + .as_ref() + .expect("Accessing stderr of a command that did not capture stderr") + .to_vec(), ) .expect("Cannot parse process stderr as UTF-8") } #[must_use] pub fn stderr_if_present(&self) -> Option { - self.stderr.as_ref().and_then(|s| String::from_utf8(s.clone()).ok()) + self.stderr.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok()) } } @@ -536,8 +542,8 @@ impl Default for CommandOutput { fn default() -> Self { Self { status: CommandStatus::Finished(ExitStatus::default()), - stdout: Some(vec![]), - stderr: Some(vec![]), + stdout: Some(Arc::from(vec![])), + stderr: Some(Arc::from(vec![])), } } } @@ -826,8 +832,15 @@ impl<'a> DeferredCommand<'a> { } => { let exec_ctx = exec_ctx.as_ref(); - let output = - Self::finish_process(process, command, stdout, stderr, executed_at, exec_ctx); + let output = Self::finish_process( + process, + command, + stdout, + stderr, + executed_at, + exec_ctx, + &fingerprint, + ); #[cfg(feature = "tracing")] drop(_span_guard); @@ -852,6 +865,7 @@ impl<'a> DeferredCommand<'a> { stderr: OutputMode, executed_at: &'a std::panic::Location<'a>, exec_ctx: &ExecutionContext, + fingerprint: &CommandFingerprint, ) -> CommandOutput { use std::fmt::Write; @@ -904,7 +918,7 @@ impl<'a> DeferredCommand<'a> { let command_str = if exec_ctx.is_verbose() { format!("{command:?}") } else { - command.fingerprint().format_short_cmd() + fingerprint.format_short_cmd() }; let action = match fail_reason { FailureReason::FailedAtRuntime(e) => {