Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: experiment with immutable strings #302

Closed
wants to merge 1 commit into from
Closed

perf: experiment with immutable strings #302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions brush-core/benches/shell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod unix {
}

async fn eval_arithmetic_expr(shell: &mut brush_core::Shell, expr: &str) {
let parsed_expr = brush_parser::arithmetic::parse(expr).unwrap();
let parsed_expr = brush_parser::arithmetic::parse(&expr.into()).unwrap();
let _ = shell.eval_arithmetic(parsed_expr).await.unwrap();
}

Expand Down Expand Up @@ -107,7 +107,7 @@ mod unix {
shell.funcs.update(
String::from("testfunc"),
Arc::new(brush_parser::ast::FunctionDefinition {
fname: String::from("testfunc"),
fname: "testfunc".into(),
body: brush_parser::ast::FunctionBody(
brush_parser::ast::CompoundCommand::BraceGroup(
brush_parser::ast::BraceGroupCommand(brush_parser::ast::CompoundList(
Expand All @@ -116,7 +116,7 @@ mod unix {
),
None,
),
source: String::from("/some/path"),
source: "/some/path".into(),
}),
);
c.bench_function("function_call", |b| {
Expand Down
2 changes: 1 addition & 1 deletion brush-core/src/arithmetic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl ExpandAndEvaluate for ast::UnexpandedArithmeticExpr {
.map_err(|_e| EvalError::FailedToExpandExpression)?;

// Now parse.
let expr = brush_parser::arithmetic::parse(&expanded_self)
let expr = brush_parser::arithmetic::parse(&expanded_self.clone().into())
.map_err(|_e| EvalError::ParseError(expanded_self))?;

// Trace if applicable.
Expand Down
2 changes: 1 addition & 1 deletion brush-core/src/builtins/complete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ impl builtins::Command for CompGenCommand {
command_name: None,
token_index: 0,
tokens: &[&brush_parser::Token::Word(
token_to_complete.to_owned(),
token_to_complete.to_owned().into(),
brush_parser::TokenLocation::default(),
)],
input_line: token_to_complete,
Expand Down
16 changes: 10 additions & 6 deletions brush-core/src/builtins/declare.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ impl DeclareCommand {
commands::CommandArg::Assignment(assignment) => {
match &assignment.name {
brush_parser::ast::AssignmentName::VariableName(var_name) => {
name = var_name.to_owned();
name = var_name.clone().into_std_string();
assigned_index = None;
}
brush_parser::ast::AssignmentName::ArrayElementName(var_name, index) => {
Expand All @@ -350,8 +350,8 @@ impl DeclareCommand {
return Err(error::Error::AssigningListToArrayMember);
}

name = var_name.to_owned();
assigned_index = Some(index.to_owned());
name = var_name.clone().into_std_string();
assigned_index = Some(index.clone().into_std_string());
}
}

Expand All @@ -360,19 +360,23 @@ impl DeclareCommand {
if let Some(index) = &assigned_index {
initial_value = Some(ShellValueLiteral::Array(ArrayLiteral(vec![(
Some(index.to_owned()),
s.value.clone(),
s.value.to_string(),
)])));
name_is_array = true;
} else {
initial_value = Some(ShellValueLiteral::Scalar(s.value.clone()));
initial_value =
Some(ShellValueLiteral::Scalar(s.value.clone().into_std_string()));
name_is_array = false;
}
}
brush_parser::ast::AssignmentValue::Array(a) => {
initial_value = Some(ShellValueLiteral::Array(ArrayLiteral(
a.iter()
.map(|(i, v)| {
(i.as_ref().map(|w| w.value.clone()), v.value.clone())
(
i.as_ref().map(|w| w.value.clone().into_std_string()),
v.value.clone().into_std_string(),
)
})
.collect(),
)));
Expand Down
9 changes: 6 additions & 3 deletions brush-core/src/builtins/export.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,16 @@ impl builtins::Command for ExportCommand {

let value = match &assignment.value {
brush_parser::ast::AssignmentValue::Scalar(s) => {
variables::ShellValueLiteral::Scalar(s.flatten())
variables::ShellValueLiteral::Scalar(s.flatten().into_std_string())
}
brush_parser::ast::AssignmentValue::Array(a) => {
variables::ShellValueLiteral::Array(variables::ArrayLiteral(
a.iter()
.map(|(k, v)| {
(k.as_ref().map(|k| k.flatten()), v.flatten())
(
k.as_ref().map(|k| k.flatten().into_std_string()),
v.flatten().into_std_string(),
)
})
.collect(),
))
Expand All @@ -78,7 +81,7 @@ impl builtins::Command for ExportCommand {

// Update the variable with the provided value and then mark it exported.
context.shell.env.update_or_add(
name,
name.as_str(),
value,
|var| {
var.export();
Expand Down
2 changes: 1 addition & 1 deletion brush-core/src/builtins/let_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl builtins::Command for LetCommand {
}

for expr in &self.exprs {
let parsed = brush_parser::arithmetic::parse(expr.as_str())?;
let parsed = brush_parser::arithmetic::parse(&expr.to_owned().into())?;
let evaluated = parsed.eval(context.shell).await?;

if evaluated == 0 {
Expand Down
8 changes: 5 additions & 3 deletions brush-core/src/builtins/unset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ impl builtins::Command for UnsetCommand {

for name in &self.names {
if unspecified || self.name_interpretation.shell_variables {
let parameter =
brush_parser::word::parse_parameter(name, &context.shell.parser_options())?;
let parameter = brush_parser::word::parse_parameter(
&name.to_owned().into(),
&context.shell.parser_options(),
)?;

let result = match parameter {
brush_parser::word::Parameter::Positional(_) => continue,
Expand All @@ -61,7 +63,7 @@ impl builtins::Command for UnsetCommand {
}
brush_parser::word::Parameter::NamedWithIndex { name, index } => {
// First evaluate the index expression.
let index_as_expr = brush_parser::arithmetic::parse(index.as_str())?;
let index_as_expr = brush_parser::arithmetic::parse(&index)?;
let evaluated_index = context.shell.eval_arithmetic(index_as_expr).await?;

context
Expand Down
10 changes: 6 additions & 4 deletions brush-core/src/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,8 +927,10 @@ impl Config {

// If the position is after the last token, then we need to insert an empty
// token for the new token to be generated.
let empty_token =
brush_parser::Token::Word(String::new(), brush_parser::TokenLocation::default());
let empty_token = brush_parser::Token::Word(
brush_parser::TokenString::new(),
brush_parser::TokenLocation::default(),
);
if completion_token_index == tokens.len() {
adjusted_tokens.push(&empty_token);
}
Expand Down Expand Up @@ -978,7 +980,7 @@ impl Config {
fn tokenize_input_for_completion(shell: &mut Shell, input: &str) -> Vec<brush_parser::Token> {
// Best-effort tokenization.
if let Ok(tokens) = brush_parser::tokenize_str_with_options(
input,
&input.to_owned().into(),
&(shell.parser_options().tokenizer_options()),
) {
return tokens;
Expand Down Expand Up @@ -1157,7 +1159,7 @@ fn simple_tokenize_by_delimiters(input: &str, delimiters: &[char]) -> Vec<brush_
let piece = piece.strip_suffix(delimiters).unwrap_or(piece);
let end: i32 = start + piece.len() as i32;
tokens.push(brush_parser::Token::Word(
piece.to_string(),
piece.into(),
brush_parser::TokenLocation {
start: brush_parser::SourcePosition {
index: start,
Expand Down
52 changes: 30 additions & 22 deletions brush-core/src/expansion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use std::borrow::Cow;
use std::cmp::min;

use brush_parser::ast;
use brush_parser::word::ParameterTransformOp;
use brush_parser::word::SubstringMatchKind;
use brush_parser::word::{ParameterTransformOp, SubstringMatchKind, WordString};
use itertools::Itertools;

use crate::arithmetic::ExpandAndEvaluate;
Expand Down Expand Up @@ -349,7 +348,7 @@ pub(crate) async fn assign_to_named_parameter(
) -> Result<(), error::Error> {
let parser_options = shell.parser_options();
let mut expander = WordExpander::new(shell);
let parameter = brush_parser::word::parse_parameter(name, &parser_options)?;
let parameter = brush_parser::word::parse_parameter(&name.to_owned().into(), &parser_options)?;
expander.assign_to_parameter(&parameter, value).await
}

Expand Down Expand Up @@ -379,7 +378,7 @@ impl<'a> WordExpander<'a> {
#[allow(clippy::ref_option)]
async fn basic_expand_opt_pattern(
&mut self,
word: &Option<String>,
word: &Option<WordString>,
) -> Result<Option<patterns::Pattern>, error::Error> {
if let Some(word) = word {
let pattern = self
Expand Down Expand Up @@ -461,7 +460,7 @@ impl<'a> WordExpander<'a> {

// Expand: tildes, parameters, command substitutions, arithmetic.
let mut expansions = vec![];
for piece in brush_parser::word::parse(brace_expanded.as_str(), &self.parser_options)? {
for piece in brush_parser::word::parse(&brace_expanded.into(), &self.parser_options)? {
let piece_expansion = self.expand_word_piece(piece.piece).await?;
expansions.push(piece_expansion);
}
Expand All @@ -481,7 +480,8 @@ impl<'a> WordExpander<'a> {
return Ok(vec![word.into()]);
}

let parse_result = brush_parser::word::parse_brace_expansions(word, &self.parser_options);
let parse_result =
brush_parser::word::parse_brace_expansions(&word.into(), &self.parser_options);
if parse_result.is_err() {
tracing::error!("failed to parse for brace expansion: {parse_result:?}");
return Ok(vec![word.into()]);
Expand Down Expand Up @@ -597,10 +597,10 @@ impl<'a> WordExpander<'a> {
) -> Result<Expansion, error::Error> {
let expansion: Expansion = match word_piece {
brush_parser::word::WordPiece::Text(s) => {
Expansion::from(ExpansionPiece::Splittable(s))
Expansion::from(ExpansionPiece::Splittable(s.into_std_string()))
}
brush_parser::word::WordPiece::SingleQuotedText(s) => {
Expansion::from(ExpansionPiece::Unsplittable(s))
Expansion::from(ExpansionPiece::Unsplittable(s.into_std_string()))
}
brush_parser::word::WordPiece::AnsiCQuotedText(s) => {
let (expanded, _) = escape::expand_backslash_escapes(
Expand Down Expand Up @@ -689,8 +689,11 @@ impl<'a> WordExpander<'a> {
}
brush_parser::word::WordPiece::BackquotedCommandSubstitution(s)
| brush_parser::word::WordPiece::CommandSubstitution(s) => {
let output_str =
commands::invoke_command_in_subshell_and_get_output(self.shell, s).await?;
let output_str = commands::invoke_command_in_subshell_and_get_output(
self.shell,
s.into_std_string(),
)
.await?;

// We trim trailing newlines, per spec.
let trimmed = output_str.trim_end_matches('\n');
Expand Down Expand Up @@ -1055,7 +1058,7 @@ impl<'a> WordExpander<'a> {
.set_case_insensitive(self.shell.options.case_insensitive_conditionals);

// If no replacement was provided, then we replace with an empty string.
let replacement = replacement.unwrap_or(String::new());
let replacement = replacement.unwrap_or(WordString::new());
let expanded_replacement = self.basic_expand_to_str(&replacement).await?;

let regex = expanded_pattern.to_regex(
Expand Down Expand Up @@ -1159,7 +1162,7 @@ impl<'a> WordExpander<'a> {

if let Some(index) = index {
self.shell.env.update_or_add_array_element(
variable_name,
variable_name.as_str(),
index,
value,
|_| Ok(()),
Expand All @@ -1168,7 +1171,7 @@ impl<'a> WordExpander<'a> {
)
} else {
self.shell.env.update_or_add(
variable_name,
variable_name.as_str(),
variables::ShellValueLiteral::Scalar(value),
|_| Ok(()),
env::EnvironmentLookup::Anywhere,
Expand All @@ -1188,7 +1191,7 @@ impl<'a> WordExpander<'a> {
let expansion = self.expand_parameter(parameter, true).await?;
let parameter_str: String = expansion.into();
let inner_parameter =
brush_parser::word::parse_parameter(parameter_str.as_str(), &self.parser_options)?;
brush_parser::word::parse_parameter(&parameter_str.into(), &self.parser_options)?;
Ok(self.try_resolve_parameter_to_variable_without_indirect(&inner_parameter))
}
}
Expand All @@ -1200,14 +1203,17 @@ impl<'a> WordExpander<'a> {
let (name, index) = match parameter {
brush_parser::word::Parameter::Positional(_)
| brush_parser::word::Parameter::Special(_) => (None, None),
brush_parser::word::Parameter::Named(name) => (Some(name.to_owned()), Some("0".into())),
brush_parser::word::Parameter::NamedWithIndex { name, index } => {
(Some(name.to_owned()), Some(index.to_owned()))
brush_parser::word::Parameter::Named(name) => {
(Some(name.clone().into_std_string()), Some("0".into()))
}
brush_parser::word::Parameter::NamedWithIndex { name, index } => (
Some(name.clone().into_std_string()),
Some(index.clone().into_std_string()),
),
brush_parser::word::Parameter::NamedWithAllIndices {
name,
concatenate: _concatenate,
} => (Some(name.to_owned()), None),
} => (Some(name.clone().into_std_string()), None),
};

let var = name
Expand All @@ -1227,8 +1233,10 @@ impl<'a> WordExpander<'a> {
Ok(expansion)
} else {
let parameter_str: String = expansion.into();
let inner_parameter =
brush_parser::word::parse_parameter(parameter_str.as_str(), &self.parser_options)?;
let inner_parameter = brush_parser::word::parse_parameter(
&parameter_str.clone().into(),
&self.parser_options,
)?;

self.expand_parameter_without_indirect(&inner_parameter)
.await
Expand Down Expand Up @@ -1325,7 +1333,7 @@ impl<'a> WordExpander<'a> {
self.basic_expand_to_str(index).await?
} else {
let index_expr = ast::UnexpandedArithmeticExpr {
value: index.to_owned(),
value: index.into(),
};
self.expand_arithmetic_expr(index_expr).await?
};
Expand Down Expand Up @@ -1637,7 +1645,7 @@ fn generate_and_combine_brace_expansions(
) -> Vec<String> {
let expansions: Vec<Vec<String>> = pieces
.into_iter()
.map(|piece| piece.generate().collect())
.map(|piece| piece.generate().map(|x| x.into_std_string()).collect())
.collect();

expansions
Expand Down
Loading
Loading