Skip to content

Commit 95a83bf

Browse files
committed
perf: short-term optimization for common-case printf
1 parent 9a03628 commit 95a83bf

File tree

3 files changed

+44
-14
lines changed

3 files changed

+44
-14
lines changed

brush-core/src/builtins/printf.rs

+36-14
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,37 @@ impl builtins::Command for PrintfCommand {
2525
&self,
2626
context: commands::ExecutionContext<'_>,
2727
) -> Result<crate::builtins::ExitCode, crate::error::Error> {
28+
let result = self.evaluate(&context)?;
29+
30+
if let Some(variable_name) = &self.output_variable {
31+
expansion::assign_to_named_parameter(context.shell, variable_name, result).await?;
32+
} else {
33+
write!(context.stdout(), "{result}")?;
34+
context.stdout().flush()?;
35+
}
36+
37+
return Ok(builtins::ExitCode::Success);
38+
}
39+
}
40+
41+
impl PrintfCommand {
42+
fn evaluate(
43+
&self,
44+
context: &commands::ExecutionContext<'_>,
45+
) -> Result<String, crate::error::Error> {
46+
// Special-case common format string: "%s".
47+
if self.format == "%s" && self.args.len() == 1 {
48+
return Ok(self.args[0].clone());
49+
}
50+
51+
self.evaluate_via_external_command(context)
52+
}
53+
54+
#[allow(clippy::unwrap_in_result)]
55+
fn evaluate_via_external_command(
56+
&self,
57+
context: &commands::ExecutionContext<'_>,
58+
) -> Result<String, crate::error::Error> {
2859
// TODO: Don't call external printf command.
2960
let mut cmd = std::process::Command::new("printf");
3061
cmd.env_clear();
@@ -39,21 +70,12 @@ impl builtins::Command for PrintfCommand {
3970
write!(context.stderr(), "{stderr}")?;
4071
context.stderr().flush()?;
4172

42-
if !output.status.success() {
43-
#[allow(clippy::cast_possible_truncation)]
44-
#[allow(clippy::cast_sign_loss)]
45-
return Ok(builtins::ExitCode::Custom(
46-
output.status.code().unwrap() as u8
47-
));
48-
}
49-
50-
if let Some(variable_name) = &self.output_variable {
51-
expansion::assign_to_named_parameter(context.shell, variable_name, stdout).await?;
73+
if output.status.success() {
74+
Ok(stdout)
5275
} else {
53-
write!(context.stdout(), "{stdout}")?;
54-
context.stdout().flush()?;
76+
Err(crate::error::Error::PrintfFailure(
77+
output.status.code().unwrap(),
78+
))
5579
}
56-
57-
return Ok(builtins::ExitCode::Success);
5880
}
5981
}

brush-core/src/error.rs

+4
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ pub enum Error {
168168
/// Bad file descriptor.
169169
#[error("bad file descriptor: {0}")]
170170
BadFileDescriptor(u32),
171+
172+
/// Printf failure
173+
#[error("printf failure: {0}")]
174+
PrintfFailure(i32),
171175
}
172176

173177
/// Convenience function for returning an error for unimplemented functionality.

brush-shell/tests/cases/builtins/printf.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
name: "Builtins: printf"
22
cases:
3+
- name: "One-variable printf"
4+
stdin: |
5+
printf "%s" "0"
6+
37
- name: "Basic printf"
48
stdin: |
59
printf "%s, %s" "Hello" "world"

0 commit comments

Comments
 (0)