From 4b8ceb1fbe618e8a443294a9075a75279d5d96b6 Mon Sep 17 00:00:00 2001 From: Matthew Paras <34500476+mattwparas@users.noreply.github.com> Date: Thu, 8 Feb 2024 21:32:51 -0800 Subject: [PATCH] Performance improvements (#162) --- Cargo.lock | 4 +- crates/steel-core/Cargo.toml | 4 +- crates/steel-core/src/compiler/code_gen.rs | 8 +- crates/steel-core/src/compiler/program.rs | 39 +- crates/steel-core/src/core/instructions.rs | 2 + crates/steel-core/src/core/labels.rs | 17 +- crates/steel-core/src/env.rs | 6 - crates/steel-core/src/primitives/lists.rs | 57 ++- crates/steel-core/src/primitives/nums.rs | 19 +- crates/steel-core/src/steel_vm/transducers.rs | 43 +- crates/steel-core/src/steel_vm/vm.rs | 417 ++++++++---------- crates/steel-core/src/values/lists.rs | 89 +++- crates/steel-gen/src/opcode.rs | 8 +- r7rs-benchmarks/nqueens.scm | 40 ++ 14 files changed, 409 insertions(+), 344 deletions(-) create mode 100644 r7rs-benchmarks/nqueens.scm diff --git a/Cargo.lock b/Cargo.lock index 0c1fcce0f..0c7b9d2c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1519,9 +1519,9 @@ dependencies = [ [[package]] name = "im-lists" -version = "0.7.1" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c07eff2c41645923382085ea8627509e5184f7a668f75d0c1e16091f7af4798" +checksum = "3e7233fb8b1ffc0b1d6033fd311a74a0443164628c62abbc871185ee95098b63" dependencies = [ "smallvec", ] diff --git a/crates/steel-core/Cargo.toml b/crates/steel-core/Cargo.toml index a41bdfdd7..850e5ec04 100644 --- a/crates/steel-core/Cargo.toml +++ b/crates/steel-core/Cargo.toml @@ -26,14 +26,12 @@ serde = { version = "1.0.193", features = ["derive", "rc"] } serde_derive = "1.0.193" bincode = "1.3.3" pretty = "0.12.1" -im-lists = "0.7.1" +im-lists = "0.8.0" strsim = "0.11.0" - quickscope = "0.2.0" lasso = { version = "0.7.2", features = ["multi-threaded", "serialize"] } once_cell = "1.18.0" fxhash = "0.2.1" -# lazy_static = "1.4.0" steel-gen = { path = "../steel-gen", version = "0.2.0" } steel-parser = { path = "../steel-parser", version = "0.6.0" } steel-derive = { path = "../steel-derive", version = "0.5.0" } diff --git a/crates/steel-core/src/compiler/code_gen.rs b/crates/steel-core/src/compiler/code_gen.rs index 2316c5d6d..ff7e3fabb 100644 --- a/crates/steel-core/src/compiler/code_gen.rs +++ b/crates/steel-core/src/compiler/code_gen.rs @@ -36,7 +36,6 @@ use crate::rvals::Result; pub(crate) static FUNCTION_ID: AtomicUsize = AtomicUsize::new(0); fn fresh_function_id() -> usize { - // println!("{:?}", FUNCTION_ID); FUNCTION_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed) } @@ -124,11 +123,11 @@ impl<'a> CodeGenerator<'a> { let value = eval_atom(syn)?; let idx = self.constant_map.add_or_get(value); + // First, lets check that the value fits self.push( LabeledInstruction::builder(OpCode::PUSHCONST) .payload(idx) - .contents(syn.clone()) - .constant(true), + .contents(syn.clone()), ); Ok(()) } @@ -651,8 +650,7 @@ impl<'a> VisitorMut for CodeGenerator<'a> { // TODO: This is a little suspect, we're doing a bunch of stuff twice // that we really don't need. In fact, we probably can get away with just... // embedding the steel val directly here. - .list_contents(crate::parser::ast::ExprKind::Quote(Box::new(quote.clone()))) - .constant(true), + .list_contents(crate::parser::ast::ExprKind::Quote(Box::new(quote.clone()))), ); Ok(()) diff --git a/crates/steel-core/src/compiler/program.rs b/crates/steel-core/src/compiler/program.rs index 6087223bf..13564035a 100644 --- a/crates/steel-core/src/compiler/program.rs +++ b/crates/steel-core/src/compiler/program.rs @@ -175,7 +175,7 @@ pub fn convert_call_globals(instructions: &mut [Instruction]) { if let TokenType::Identifier(ident) = ident.ty { match ident { - _ if ident == *PRIM_CONS_SYMBOL => { + _ if ident == *PRIM_CONS_SYMBOL && arity == 2 => { if let Some(x) = instructions.get_mut(i) { x.op_code = OpCode::CONS; x.payload_size = 2; @@ -192,34 +192,55 @@ pub fn convert_call_globals(instructions: &mut [Instruction]) { // continue; // } // } - _ if ident == *BOX || ident == *PRIM_BOX => { + _ if ident == *BOX || ident == *PRIM_BOX && arity == 1 => { if let Some(x) = instructions.get_mut(i) { x.op_code = OpCode::NEWBOX; continue; } } - _ if ident == *UNBOX || ident == *PRIM_UNBOX => { + _ if ident == *UNBOX || ident == *PRIM_UNBOX && arity == 1 => { if let Some(x) = instructions.get_mut(i) { x.op_code = OpCode::UNBOX; continue; } } - _ if ident == *SETBOX || ident == *PRIM_SETBOX => { + _ if ident == *SETBOX || ident == *PRIM_SETBOX && arity == 1 => { if let Some(x) = instructions.get_mut(i) { x.op_code = OpCode::SETBOX; continue; } } - _ if ident == *PRIM_CAR => { + _ if ident == *PRIM_CAR && arity == 1 => { if let Some(x) = instructions.get_mut(i) { x.op_code = OpCode::CAR; continue; } } + _ if ident == *PRIM_CDR && arity == 1 => { + if let Some(x) = instructions.get_mut(i) { + x.op_code = OpCode::CDR; + continue; + } + } + + _ if ident == *PRIM_NOT && arity == 1 => { + if let Some(x) = instructions.get_mut(i) { + x.op_code = OpCode::NOT; + continue; + } + } + + _ if ident == *PRIM_NULL && arity == 1 => { + if let Some(x) = instructions.get_mut(i) { + x.op_code = OpCode::NULL; + continue; + } + } + // _ if ident == *CDR_SYMBOL || ident == *PRIM_CAR => { // if let Some(x) = instructions.get_mut(i) { // x.op_code = OpCode::CAR; @@ -347,9 +368,12 @@ define_primitive_symbols! { (PRIM_DIV, DIV) => "/", (PRIM_STAR, STAR) => "*", (PRIM_EQUAL, EQUAL) => "equal?", + (PRIM_NUM_EQUAL, NUM_EQUAL) => "=", (PRIM_LTE, LTE) => "<=", (PRIM_CAR, CAR_SYMBOL) => "car", (PRIM_CDR, CDR_SYMBOL) => "cdr", + (PRIM_NOT, NOT_SYMBOL) => "not", + (PRIM_NULL, NULL_SYMBOL) => "null?", } define_symbols! { @@ -435,9 +459,11 @@ pub fn inline_num_operations(instructions: &mut [Instruction]) { let replaced = match *ident { x if x == *PRIM_PLUS && *payload_size == 2 => Some(OpCode::BINOPADD), x if x == *PRIM_PLUS && *payload_size > 0 => Some(OpCode::ADD), + // x if x == *PRIM_MINUS && *payload_size == 2 => Some(OpCode::BINOPSUB), x if x == *PRIM_MINUS && *payload_size > 0 => Some(OpCode::SUB), x if x == *PRIM_DIV && *payload_size > 0 => Some(OpCode::DIV), x if x == *PRIM_STAR && *payload_size > 0 => Some(OpCode::MUL), + x if x == *PRIM_NUM_EQUAL && *payload_size == 2 => Some(OpCode::NUMEQUAL), x if x == *PRIM_EQUAL && *payload_size > 0 => Some(OpCode::EQUAL), x if x == *PRIM_LTE && *payload_size > 0 => Some(OpCode::LTE), _ => None, @@ -1116,9 +1142,8 @@ fn extract_spans( DenseInstruction::new( i.op_code, i.payload_size.try_into().unwrap_or_else(|_| { - // println!("{:?}", len); println!("{:?}", i); - panic!("Uh oh ") + panic!("Unable to lower instruction to bytecode!") }), ) }) diff --git a/crates/steel-core/src/core/instructions.rs b/crates/steel-core/src/core/instructions.rs index 159e39f14..8c7c29645 100644 --- a/crates/steel-core/src/core/instructions.rs +++ b/crates/steel-core/src/core/instructions.rs @@ -117,6 +117,8 @@ pub fn disassemble(instructions: &[Instruction]) -> String { pub struct DenseInstruction { pub op_code: OpCode, // Function IDs need to be interned _again_ before patched into the code? + // Also: We should be able to get away with a u16 here. Just grab places where u16 + // won't fit and convert to something else. pub payload_size: u32, } diff --git a/crates/steel-core/src/core/labels.rs b/crates/steel-core/src/core/labels.rs index ff2b97da2..801af8e9c 100644 --- a/crates/steel-core/src/core/labels.rs +++ b/crates/steel-core/src/core/labels.rs @@ -23,7 +23,7 @@ pub enum Expr { } #[derive(Clone, Debug)] -pub struct LabeledInstruction { +pub struct CompactInstruction { pub op_code: OpCode, pub payload_size: usize, pub contents: Option, @@ -32,6 +32,15 @@ pub struct LabeledInstruction { pub constant: bool, } +#[derive(Clone, Debug)] +pub struct LabeledInstruction { + pub op_code: OpCode, + pub payload_size: usize, + pub contents: Option, + pub tag: Option