Commit c441ba3a authored by Jack Fransham's avatar Jack Fransham Committed by Michael Aaron Murphy
Browse files

Reduce allocations and cloning (#271)

This is a pretty huge rework, replacing String with a custom SmallString
based on a SmallVec of bytes. Replacing newlines with spaces is now done
in-place, and most (for some value of "most") strings can now be stored
on the stack. This should reduce allocator load, but it also improves
cache locality. Whatever the real reason is, each of these changes
provides a real, measurable speed boost, and they combine to give an
improvement of something like 20-25%, meaning we're now 30-35% faster
than Dash. This is a breaking change, replacing some uses of String with
SmallString or &str in the public API, but I'm pretty sure no-one's
hooking into our public API anyway so that should be OK.
parent 4edf5193
......@@ -24,6 +24,8 @@ liner = "0.1.4"
peg-syntax-ext = "0.4"
permutate = "0.2"
unicode-segmentation = "1.1"
smallvec = "0.3.3"
smallstring = { path = "src/smallstring" }
[target.'cfg(all(unix, not(target_os = "redox")))'.dependencies]
users = "0.5.1"
......
use std::ascii::AsciiExt;
use std::ops::DerefMut;
// TODO: These could be generalised to work on non-ASCII characters (and even
// strings!) as long as the byte size of the needle and haystack match.
pub trait AsciiReplaceInPlace {
fn ascii_replace_in_place(&mut self, needle: char, haystack: char);
}
pub trait AsciiReplace: Sized {
fn ascii_replace(self, needle: char, haystack: char) -> Self;
}
impl<T: DerefMut<Target=str>> AsciiReplace for T {
fn ascii_replace(mut self, needle: char, haystack: char) -> Self {
self.ascii_replace_in_place(needle, haystack);
self
}
}
impl AsciiReplaceInPlace for str {
// I tried replacing these `assert!` calls with `debug_assert!` but it looks
// like they get const-folded away anyway since it doesn't affect the speed
fn ascii_replace_in_place(&mut self, needle: char, haystack: char) {
assert!(
needle.is_ascii(),
"AsciiReplace functions can only be used for ascii characters"
);
assert!(
haystack.is_ascii(),
"AsciiReplace functions can only be used for ascii characters"
);
let (needle, haystack): (u32, u32) = (needle.into(), haystack.into());
let (needle, haystack) = (needle as u8, haystack as u8);
// This is safe because we verify that we don't modify non-ascii bytes
let mut_bytes = unsafe { self.as_bytes_mut() };
for chr in mut_bytes.iter_mut() {
if *chr == needle {
*chr = haystack;
}
}
}
}
......@@ -11,6 +11,7 @@ pub enum Token {
Exponent,
OpenParen,
CloseParen,
// TODO: Don't pass around a string when we can pass around a number
Number(String),
}
......@@ -287,7 +288,7 @@ fn eval(input: &str) -> Result<String, CalcError> {
tokenize(input).and_then(|x| parse(&x))
}
pub fn calc(args: &[String]) -> Result<(), String> {
pub fn calc(args: &[&str]) -> Result<(), String> {
let stdout = io::stdout();
let mut stdout = stdout.lock();
if !args.is_empty() {
......
......@@ -36,12 +36,12 @@ OPTIONS
\v vertical tab (VT)
"#; /* @MANEND */
pub fn echo(args: &[String]) -> Result<(), io::Error> {
pub fn echo(args: &[&str]) -> Result<(), io::Error> {
let mut flags = 0u8;
let mut data: Vec<&str> = vec![];
for arg in args {
match arg.as_str() {
match *arg {
"--help" => flags |= HELP,
"--escape" => flags |= ESCAPE,
"--no-newline" => flags |= NO_NEWLINE,
......
......@@ -2,8 +2,9 @@ use shell::flow_control::Function;
use fnv::FnvHashMap;
use shell::status::*;
use std::io::{self, Write};
use types::Identifier;
fn print_functions(functions: &FnvHashMap<String, Function>) {
fn print_functions(functions: &FnvHashMap<Identifier, Function>) {
let stdout = io::stdout();
let stdout = &mut stdout.lock();
let _ = writeln!(stdout, "# Functions");
......@@ -17,7 +18,7 @@ fn print_functions(functions: &FnvHashMap<String, Function>) {
}
}
pub fn fn_(functions: &mut FnvHashMap<String, Function>) -> i32
pub fn fn_(functions: &mut FnvHashMap<Identifier, Function>) -> i32
{
print_functions(functions);
SUCCESS
......
......@@ -31,7 +31,7 @@ use shell::status::*;
/// let my_command = Builtin {
/// name: "my_command",
/// help: "Describe what my_command does followed by a newline showing usage",
/// main: box|args: &[String], &mut Shell| -> i32 {
/// main: box|args: &[&str], &mut Shell| -> i32 {
/// println!("Say 'hello' to my command! :-D");
/// }
/// }
......@@ -39,20 +39,21 @@ use shell::status::*;
pub struct Builtin {
pub name: &'static str,
pub help: &'static str,
pub main: Box<Fn(&[String], &mut Shell) -> i32>,
pub main: Box<Fn(&[&str], &mut Shell) -> i32>,
}
impl Builtin {
/// Return the map from command names to commands
pub fn map() -> FnvHashMap<&'static str, Self> {
let mut commands: FnvHashMap<&str, Self> = FnvHashMap::with_capacity_and_hasher(32, Default::default());
let mut commands: FnvHashMap<&str, Self> =
FnvHashMap::with_capacity_and_hasher(32, Default::default());
/* Directories */
commands.insert("cd",
Builtin {
name: "cd",
help: "Change the current directory\n cd <path>",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
match shell.directory_stack.cd(args, &shell.variables) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -69,7 +70,7 @@ impl Builtin {
Builtin {
name: "dirs",
help: "Display the current directory stack",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
shell.directory_stack.dirs(args)
},
});
......@@ -78,7 +79,7 @@ impl Builtin {
Builtin {
name: "pushd",
help: "Push a directory to the stack",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
match shell.directory_stack.pushd(args, &shell.variables) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -95,7 +96,7 @@ impl Builtin {
Builtin {
name: "popd",
help: "Pop a directory from the stack",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
match shell.directory_stack.popd(args) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -113,7 +114,7 @@ impl Builtin {
Builtin {
name: "alias",
help: "View, set or unset aliases",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
alias(&mut shell.variables, args)
},
});
......@@ -122,7 +123,7 @@ impl Builtin {
Builtin {
name: "drop",
help: "Delete an alias",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
drop_alias(&mut shell.variables, args)
},
});
......@@ -132,7 +133,7 @@ impl Builtin {
Builtin {
name: "export",
help: "Set an environment variable",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
export_variable(&mut shell.variables, args)
}
});
......@@ -141,7 +142,7 @@ impl Builtin {
Builtin {
name: "fn",
help: "Print list of functions",
main: box |_: &[String], shell: &mut Shell| -> i32 {
main: box |_: &[&str], shell: &mut Shell| -> i32 {
fn_(&mut shell.functions)
},
});
......@@ -150,7 +151,7 @@ impl Builtin {
Builtin {
name: "read",
help: "Read some variables\n read <variable>",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
shell.variables.read(args)
},
});
......@@ -159,7 +160,7 @@ impl Builtin {
Builtin {
name: "drop",
help: "Delete a variable",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
drop_variable(&mut shell.variables, args)
},
});
......@@ -169,7 +170,7 @@ impl Builtin {
Builtin {
name: "set",
help: "Set or unset values of shell options and positional parameters.",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
set::set(args, shell)
},
});
......@@ -178,7 +179,7 @@ impl Builtin {
Builtin {
name: "eval",
help: "evaluates the evaluated expression",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
let evaluated_command = args[1..].join(" ");
let mut buffer = QuoteTerminator::new(evaluated_command);
if buffer.check_termination() {
......@@ -197,7 +198,7 @@ impl Builtin {
Builtin {
name: "exit",
help: "To exit the curent session",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
process::exit(args.get(1).and_then(|status| status.parse::<i32>().ok())
.unwrap_or(shell.previous_status))
},
......@@ -207,7 +208,7 @@ impl Builtin {
Builtin {
name: "history",
help: "Display a log of all commands previously executed",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
shell.print_history(args)
},
});
......@@ -216,7 +217,7 @@ impl Builtin {
Builtin {
name: "source",
help: "Evaluate the file following the command or re-initialize the init file",
main: box |args: &[String], shell: &mut Shell| -> i32 {
main: box |args: &[&str], shell: &mut Shell| -> i32 {
match source(shell, args) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -234,7 +235,7 @@ impl Builtin {
Builtin {
name: "echo",
help: "Display a line of text",
main: box |args: &[String], _: &mut Shell| -> i32 {
main: box |args: &[&str], _: &mut Shell| -> i32 {
match echo(args) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -251,7 +252,7 @@ impl Builtin {
Builtin {
name: "test",
help: "Performs tests on files and text",
main: box |args: &[String], _: &mut Shell| -> i32 {
main: box |args: &[&str], _: &mut Shell| -> i32 {
match test(args) {
Ok(true) => SUCCESS,
Ok(false) => FAILURE,
......@@ -269,7 +270,7 @@ impl Builtin {
Builtin {
name: "calc",
help: "Calculate a mathematical expression",
main: box |args: &[String], _: &mut Shell| -> i32 {
main: box |args: &[&str], _: &mut Shell| -> i32 {
match calc::calc(&args[1..]) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -286,7 +287,7 @@ impl Builtin {
Builtin {
name: "time",
help: "Measures the time to execute an external command",
main: box |args: &[String], _: &mut Shell| -> i32 {
main: box |args: &[&str], _: &mut Shell| -> i32 {
match time::time(&args[1..]) {
Ok(()) => SUCCESS,
Err(why) => {
......@@ -303,7 +304,7 @@ impl Builtin {
Builtin {
name: "true",
help: "Do nothing, successfully",
main: box |_: &[String], _: &mut Shell| -> i32 {
main: box |_: &[&str], _: &mut Shell| -> i32 {
SUCCESS
},
});
......@@ -312,7 +313,7 @@ impl Builtin {
Builtin {
name: "false",
help: "Do nothing, unsuccessfully",
main: box |_: &[String], _: &mut Shell| -> i32 {
main: box |_: &[&str], _: &mut Shell| -> i32 {
FAILURE
},
});
......@@ -328,12 +329,12 @@ impl Builtin {
name: "help",
help: "Display helpful information about a given command, or list \
commands if none specified\n help <command>",
main: box move |args: &[String], _: &mut Shell| -> i32 {
main: box move |args: &[&str], _: &mut Shell| -> i32 {
let stdout = io::stdout();
let mut stdout = stdout.lock();
if let Some(command) = args.get(1) {
if command_helper.contains_key(command.as_str()) {
if let Some(help) = command_helper.get(command.as_str()) {
if command_helper.contains_key(command) {
if let Some(help) = command_helper.get(command) {
let _ = stdout.write_all(help.as_bytes());
let _ = stdout.write_all(b"\n");
}
......
......@@ -29,7 +29,7 @@ enum PositionalArgs {
use self::PositionalArgs::*;
pub fn set(args: &[String], shell: &mut Shell) -> i32 {
pub fn set(args: &[&str], shell: &mut Shell) -> i32 {
let stdout = io::stdout();
let mut stdout = stdout.lock();
let mut args_iter = args.iter();
......@@ -69,7 +69,12 @@ pub fn set(args: &[String], shell: &mut Shell) -> i32 {
None => (),
Some(kind) => {
let command: String = shell.variables.get_array("args").unwrap()[0].to_owned();
let arguments: Vec<String> = iter::once(command).chain(args_iter.cloned()).collect();
// This used to take a `&[String]` but cloned them all, so although
// this is non-ideal and could probably be better done with `Rc`, it
// hasn't got any slower.
let arguments = iter::once(command)
.chain(args_iter.map(|i| i.to_string()))
.collect();
match kind {
UnsetIfNone => shell.variables.set_array("args", arguments),
RetainIfNone => if arguments.len() != 1 {
......
......@@ -3,7 +3,7 @@ use std::io::Read;
use shell::{Shell, FlowLogic};
/// Evaluates the given file and returns 'SUCCESS' if it succeeds.
pub fn source(shell: &mut Shell, arguments: &[String]) -> Result<(), String> {
pub fn source(shell: &mut Shell, arguments: &[&str]) -> Result<(), String> {
match arguments.get(1) {
Some(argument) => {
if let Ok(mut file) = File::open(&argument) {
......
......@@ -4,6 +4,7 @@ use std::path::Path;
use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt};
use std::time::SystemTime;
use std::error::Error;
use smallstring::SmallString;
const MAN_PAGE: &'static str = /* @MANSTART{test} */ r#"NAME
test - perform tests on files and text
......@@ -110,7 +111,7 @@ AUTHOR
Written by Michael Murphy.
"#; /* @MANEND */
pub fn test(args: &[String]) -> Result<bool, String> {
pub fn test(args: &[&str]) -> Result<bool, String> {
let stdout = io::stdout();
let mut buffer = BufWriter::new(stdout.lock());
......@@ -118,9 +119,9 @@ pub fn test(args: &[String]) -> Result<bool, String> {
evaluate_arguments(arguments, &mut buffer)
}
fn evaluate_arguments(arguments: &[String], buffer: &mut BufWriter<io::StdoutLock>) -> Result<bool, String> {
fn evaluate_arguments(arguments: &[&str], buffer: &mut BufWriter<io::StdoutLock>) -> Result<bool, String> {
if let Some(arg) = arguments.first() {
if arg.as_str() == "--help" {
if *arg == "--help" {
buffer.write_all(MAN_PAGE.as_bytes()).map_err(|x| x.description().to_owned())?;
buffer.flush().map_err(|x| x.description().to_owned())?;
......@@ -134,7 +135,7 @@ fn evaluate_arguments(arguments: &[String], buffer: &mut BufWriter<io::StdoutLoc
// If no argument was given, return `SUCCESS`
arguments.get(1).map_or(Ok(true), |argument| {
// match the correct function to the associated flag
Ok(match_flag_argument(flag, argument.as_str()))
Ok(match_flag_argument(flag, argument))
})
})
},
......@@ -142,8 +143,8 @@ fn evaluate_arguments(arguments: &[String], buffer: &mut BufWriter<io::StdoutLoc
// If there is no operator, check if the first argument is non-zero
arguments.get(1).map_or(Ok(string_is_nonzero(arg)), |operator| {
// If there is no right hand argument, a condition was expected
let right_arg = arguments.get(2).ok_or_else(|| String::from("parse error: condition expected"))?;
evaluate_expression(arg.as_str(), operator.as_str(), right_arg.as_str())
let right_arg = arguments.get(2).ok_or_else(|| SmallString::from("parse error: condition expected"))?;
evaluate_expression(arg, operator, right_arg)
})
},
};
......@@ -370,43 +371,43 @@ fn test_integers_arguments() {
let mut buffer = BufWriter::new(stdout.lock());
// Equal To
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-eq"), String::from("10")],
assert_eq!(evaluate_arguments(&["10", "-eq", "10"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-eq"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-eq", "5"],
&mut buffer), Ok(false));
// Greater Than or Equal To
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-ge"), String::from("10")],
assert_eq!(evaluate_arguments(&["10", "-ge", "10"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-ge"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-ge", "5"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-ge"), String::from("10")],
assert_eq!(evaluate_arguments(&["5", "-ge", "10"],
&mut buffer), Ok(false));
// Less Than or Equal To
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-le"), String::from("5")],
assert_eq!(evaluate_arguments(&["5", "-le", "5"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-le"), String::from("10")],
assert_eq!(evaluate_arguments(&["5", "-le", "10"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-le"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-le", "5"],
&mut buffer), Ok(false));
// Less Than
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-lt"), String::from("10")],
assert_eq!(evaluate_arguments(&["5", "-lt", "10"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-lt"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-lt", "5"],
&mut buffer), Ok(false));
// Greater Than
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-gt"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-gt", "5"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-gt"), String::from("10")],
assert_eq!(evaluate_arguments(&["5", "-gt", "10"],
&mut buffer), Ok(false));
// Not Equal To
assert_eq!(evaluate_arguments(&[String::from("10"), String::from("-ne"), String::from("5")],
assert_eq!(evaluate_arguments(&["10", "-ne", "5"],
&mut buffer), Ok(true));
assert_eq!(evaluate_arguments(&[String::from("5"), String::from("-ne"), String::from("5")],
assert_eq!(evaluate_arguments(&["5", "-ne", "5"],
&mut buffer), Ok(false));
}
......
......@@ -18,12 +18,12 @@ OPTIONS
display this help and exit
"#;
pub fn time(args: &[String]) -> Result<(), String> {
pub fn time(args: &[&str]) -> Result<(), String> {
let stdout = stdout();
let mut stdout = stdout.lock();
for arg in args {
if arg.as_str() == "-h" || arg == "--help" {
if *arg == "-h" || *arg == "--help" {
return match stdout.write_all(MAN_PAGE.as_bytes()).and_then(|_| stdout.flush()) {
Ok(_) => Ok(()),
Err(err) => Err(err.description().to_owned())
......
// TODO: Move into grammar
use fnv::FnvHashMap;
use std::env;
use std::io::{self, Write};
use types::*;
use shell::status::*;
use shell::variables::Variables;
fn print_list(list: &FnvHashMap<String, String>) {
fn print_list(list: &VariableContext) {
let stdout = io::stdout();
let stdout = &mut stdout.lock();
......@@ -20,12 +20,12 @@ fn print_list(list: &FnvHashMap<String, String>) {
}
enum Binding {
InvalidKey(String),
InvalidKey(Identifier),
ListEntries,
KeyOnly(String),
KeyValue(String, String),
Math(String, Operator, f32),
MathInvalid(String)
KeyOnly(Identifier),
KeyValue(Identifier, Value),
Math(Identifier, Operator, f32),
MathInvalid(Value)
}
enum Operator {
......@@ -36,14 +36,11 @@ enum Operator {
}
/// Parses let bindings, `let VAR = KEY`, returning the result as a `(key, value)` tuple.
fn parse_assignment<I: IntoIterator>(args: I)
-> Binding where I::Item: AsRef<str>
{
fn parse_assignment<'a, S: AsRef<str> + 'a>(args: &[S]) -> Binding {
// Write all the arguments into a single `String`
let arguments = args.into_iter().skip(1).fold(String::new(), |a, b| a + " " + b.as_ref());
// Create a character iterator from the arguments.
let mut char_iter = arguments.chars();
let mut char_iter = args.iter().skip(1)
.map(|arg| arg.as_ref().chars())
.flat_map(|chars| chars);
// Find the key and advance the iterator until the equals operator is found.
let mut key = "".to_owned();
......@@ -93,10 +90,12 @@ fn parse_assignment<I: IntoIterator>(args: I)
}
}
let key: Identifier = key.into();
if !found_key && key.is_empty() {
Binding::ListEntries
} else {
let value = char_iter.skip_while(|&x| x == ' ').collect::<String>();
let value: Value = char_iter.skip_while(|&x| x == ' ').collect();
if value.is_empty() {
Binding::KeyOnly(key)
} else if !Variables::is_valid_variable_name(&key) {
......@@ -117,9 +116,7 @@ fn parse_assignment<I: IntoIterator>(args: I)
/// The `alias` command will define an alias for another command, and thus may be used as a
/// command itself.
pub fn alias<I: IntoIterator>(vars: &mut Variables, args: I) -> i32
where I::Item: AsRef<str>
{
pub fn alias<'a, S: AsRef<str> + 'a>(vars: &mut Variables, args: &[S]) -> i32 {
match parse_assignment(args) {
Binding::InvalidKey(key) => {
let stderr = io::stderr();
......@@ -173,6 +170,7 @@ pub fn drop_variable<I: IntoIterator>(vars: &mut Variables, args: I) -> i32
let _ = writeln!(&mut stderr.lock(), "ion: you must specify a variable name");
return FAILURE;
}
for variable in args.iter().skip(1) {
if vars.unset_var(variable.as_ref()).is_none() {
let stderr = io::stderr();
......@@ -180,14 +178,14 @@ pub fn drop_variable<I: IntoIterator>(vars: &mut Variables, args: I) -> i32
return FAILURE;
}
}
SUCCESS
}
/// Exporting a variable sets that variable as a global variable in the system.
/// Global variables can be accessed by other programs running on the system.
pub fn export_variable<I: IntoIterator>(vars: &mut Variables, args: I) -> i32
where I::Item: AsRef<str>
pub fn export_variable<'a, S: AsRef<str> + 'a>(vars: &mut Variables, args: &[S]) -> i32
{
match parse_assignment(args) {
Binding::InvalidKey(key) => {
......@@ -206,7 +204,7 @@ pub fn export_variable<I: IntoIterator>(vars: &mut Variables, args: I) -> i32
}
},
Binding::Math(key, operator, increment) => {
let value = vars.get_var(&key).unwrap_or_else(|| "".to_owned());
let value = vars.get_var(&key).unwrap_or_else(|| "".into());
match value.parse::<f32>() {
Ok(old_value) => match operator {
Operator::Plus => env::set_var(key, (old_value + increment)