Skip to content

PERF? #77

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

Merged
timfennis merged 5 commits into from
Dec 4, 2025
Merged

PERF? #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
587aa10
PERF?
timfennis Dec 4, 2025
4aaf04e
clean
timfennis Dec 4, 2025
2bef921
Inline EnvironmentRef type for clarity
timfennis Dec 4, 2025
c6440b3
not really a refactor but easier to read code
timfennis Dec 4, 2025
8bc958c
Make more simple
timfennis Dec 4, 2025
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
49 changes: 25 additions & 24 deletions ndc_lib/src/interpreter/environment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ use std::fmt::Formatter;
use std::io::{Stdout, Write, stdout};
use std::rc::Rc;

pub type EnvironmentRef = Rc<RefCell<Environment>>;

pub struct RootEnvironment {
pub output: Box<dyn InterpreterOutput>,
// These are global values
Expand All @@ -19,8 +17,8 @@ pub struct RootEnvironment {

pub struct Environment {
root: Rc<RefCell<RootEnvironment>>,
parent: Option<EnvironmentRef>,
values: Vec<Rc<RefCell<Value>>>,
parent: Option<Rc<RefCell<Environment>>>,
values: Vec<Value>,
}

impl fmt::Debug for Environment {
Expand Down Expand Up @@ -62,7 +60,7 @@ impl Environment {
let mut env = Self {
root: Rc::new(RefCell::new(root)),
parent: None,
values: Vec::new(),
values: Default::default(),
};

crate::stdlib::register(&mut env);
Expand Down Expand Up @@ -96,21 +94,23 @@ impl Environment {
// types during the resolver pass
if let Value::Function(value_to_insert) = &value {
if let Some(existing_value) = self.values.get(slot) {
let existing_value = &*existing_value.borrow();
if let Value::Function(func) = existing_value {
func.borrow_mut().merge(&mut value_to_insert.borrow_mut())
}
} else {
self.values.insert(slot, Rc::new(RefCell::new(value)));
self.values.push(value);
// self.values.insert(slot, value);
}

return;
}

if self.values.len() > slot {
self.values[slot] = Rc::new(RefCell::new(value))
self.values[slot] = value
} else {
self.values.insert(slot, Rc::new(RefCell::new(value)))
debug_assert!(slot == self.values.len());
// self.values.insert(slot, value) // TODO: push should work here right
self.values.push(value);
}
}

Expand Down Expand Up @@ -153,40 +153,41 @@ impl Environment {
gb.push(OverloadedFunction::from_multiple(vec![new_function]))
}

fn get_slot(&self, depth: usize, slot: usize) -> Rc<RefCell<Value>> {
fn get_copy_from_slot(&self, depth: usize, slot: usize) -> Value {
if depth == 0 {
self.values[slot].clone()
} else {
self.parent
.clone()
.expect("expected parent env did not exist")
.borrow()
.get_slot(depth - 1, slot)
.get_copy_from_slot(depth - 1, slot)
}
}

#[must_use]
pub fn get(&self, var: ResolvedVar) -> Rc<RefCell<Value>> {
pub fn get(&self, var: ResolvedVar) -> Value {
match var {
ResolvedVar::Captured { depth, slot } => self.get_slot(depth, slot),
ResolvedVar::Global { slot } => Rc::new(RefCell::new(Value::function(
self.root.borrow().global_functions[slot].clone(),
))),
ResolvedVar::Captured { depth, slot } => self.get_copy_from_slot(depth, slot),
ResolvedVar::Global { slot } => {
Value::function(self.root.borrow().global_functions[slot].clone())
}
}
}

/// Takes the named variable from memory and leaves `Value::unit()` in its place
#[must_use]
pub fn take(&self, var: ResolvedVar) -> Option<Value> {
pub fn take(&mut self, var: ResolvedVar) -> Option<Value> {
match var {
ResolvedVar::Captured { depth: 0, slot } => Some(std::mem::replace(
&mut *self.values[slot].borrow_mut(),
self.values.get_mut(slot).expect("slot can't be empty"),
Value::unit(),
)),
ResolvedVar::Captured { depth, slot } => self
.parent
.clone()
.expect("expected parent env did not exist")
.borrow()
.borrow_mut()
.take(ResolvedVar::Captured {
depth: depth - 1,
slot,
Expand All @@ -195,13 +196,13 @@ impl Environment {
}
}

pub fn new_scope(parent: &EnvironmentRef) -> EnvironmentRef {
pub fn new_scope(parent: &Rc<RefCell<Self>>) -> Self {
let root_ref = Rc::clone(&parent.borrow().root);
Rc::new(RefCell::new(Self {
parent: Some(Rc::clone(parent)),
Self {
parent: Some(parent.clone()),
root: root_ref,
values: Vec::new(),
}))
values: Default::default(),
}
}
}

Expand Down
9 changes: 5 additions & 4 deletions ndc_lib/src/interpreter/evaluate/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@
//! +----------------+-----+----+----+----+----+----+----+----+----+----+

use super::{EvaluationError, IntoEvaluationResult, evaluate_expression};
use crate::interpreter::environment::Environment;
use crate::{
ast::{Expression, ExpressionLocation},
interpreter::{
environment::EnvironmentRef, function::FunctionCarrier, sequence::Sequence, value::Value,
},
interpreter::{function::FunctionCarrier, sequence::Sequence, value::Value},
lexer::Span,
};
use itertools::Itertools;
use std::cell::RefCell;
use std::cmp::min;
use std::ops::IndexMut;
use std::rc::Rc;

#[derive(Clone)]
pub enum EvaluatedIndex {
Expand Down Expand Up @@ -63,7 +64,7 @@ impl EvaluatedIndex {

pub(crate) fn evaluate_as_index(
expression_location: &ExpressionLocation,
environment: &mut EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
Copy link
Owner Author

@timfennis timfennis Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is het zelfde, maar EnvironmentRef was een alias (ik vind dat minder leesbaar)

) -> Result<EvaluatedIndex, FunctionCarrier> {
let (range_start, range_end, inclusive) = match expression_location.expression {
Expression::RangeExclusive {
Expand Down
38 changes: 17 additions & 21 deletions ndc_lib/src/interpreter/evaluate/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::ast::{Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue};
use crate::hash_map::HashMap;
use crate::interpreter::environment::{Environment, EnvironmentRef};
use crate::interpreter::environment::Environment;
use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction};
use crate::interpreter::int::Int;
use crate::interpreter::iterator::mut_value_to_iterator;
Expand All @@ -22,7 +22,7 @@ mod index;
#[allow(clippy::too_many_lines)]
pub(crate) fn evaluate_expression(
expression_location: &ExpressionLocation,
environment: &mut EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
) -> EvaluationResult {
let span = expression_location.span;
let literal: Value = match &expression_location.expression {
Expand All @@ -40,8 +40,6 @@ pub(crate) fn evaluate_expression(
environment
.borrow()
.get(resolved.expect("identifier was not resolved before execution"))
.borrow()
.clone()
}
Expression::VariableDeclaration { l_value, value } => {
let value = evaluate_expression(value, environment)?;
Expand Down Expand Up @@ -106,15 +104,14 @@ pub(crate) fn evaluate_expression(

while let Some((resolved_op, modified_in_place)) = operations_to_try.next() {
let operation = environment.borrow().get(resolved_op);
let operation_val = &*operation.borrow();

let Value::Function(func) = operation_val else {
let Value::Function(func) = operation else {
unreachable!(
"the resolver pass should have guaranteed that the operation points to a function"
);
};

let result = match call_function(func, &mut arguments, environment, span) {
let result = match call_function(&func, &mut arguments, environment, span) {
Err(FunctionCarrier::FunctionNotFound)
if operations_to_try.peek().is_none() =>
{
Expand Down Expand Up @@ -162,8 +159,7 @@ pub(crate) fn evaluate_expression(
while let Some((resolved_operation, modified_in_place)) =
operations_to_try.next()
{
let operation = environment.borrow().get(resolved_operation);
let operation_val = &*operation.borrow();
let operation_val = environment.borrow().get(resolved_operation);

let Value::Function(func) = operation_val else {
unreachable!(
Expand All @@ -172,7 +168,7 @@ pub(crate) fn evaluate_expression(
};

let result = match call_function(
func,
&func,
&mut [value_at_index.clone(), right_value.clone()],
environment,
span,
Expand Down Expand Up @@ -218,11 +214,11 @@ pub(crate) fn evaluate_expression(
}
}
Expression::Block { statements } => {
let mut local_scope = Environment::new_scope(environment);
let local_scope = Rc::new(RefCell::new(Environment::new_scope(environment)));

let mut value = Value::unit();
for stm in statements {
value = evaluate_expression(stm, &mut local_scope)?;
value = evaluate_expression(stm, &local_scope)?;
}

drop(local_scope);
Expand Down Expand Up @@ -655,7 +651,7 @@ pub(crate) fn evaluate_expression(

fn produce_default_value(
default: &Value,
environment: &EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
span: Span,
) -> EvaluationResult {
match default {
Expand All @@ -678,7 +674,7 @@ fn produce_default_value(
fn declare_or_assign_variable(
l_value: &Lvalue,
value: Value,
environment: &mut EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
span: Span,
) -> EvaluationResult {
match l_value {
Expand Down Expand Up @@ -870,7 +866,7 @@ where
fn call_function(
function: &Rc<RefCell<OverloadedFunction>>,
evaluated_args: &mut [Value],
environment: &EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
span: Span,
) -> EvaluationResult {
let result = function.borrow().call(evaluated_args, environment);
Expand All @@ -891,7 +887,7 @@ fn call_function(

fn execute_body(
body: &ForBody,
environment: &mut EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
result: &mut Vec<Value>,
) -> EvaluationResult {
match body {
Expand Down Expand Up @@ -924,7 +920,7 @@ fn execute_for_iterations(
iterations: &[ForIteration],
body: &ForBody,
out_values: &mut Vec<Value>,
environment: &mut EnvironmentRef,
environment: &Rc<RefCell<Environment>>,
span: Span,
) -> Result<Value, FunctionCarrier> {
let Some((cur, tail)) = iterations.split_first() else {
Expand All @@ -944,17 +940,17 @@ fn execute_for_iterations(
// ```
// With the current implementation with a new scope declared for every iteration this produces 10 functions
// each with their own scope and their own version of `i`, this might potentially be a bit slower though
let mut scope = Environment::new_scope(environment);
declare_or_assign_variable(l_value, r_value, &mut scope, span)?;
let scope = Rc::new(RefCell::new(Environment::new_scope(environment)));
declare_or_assign_variable(l_value, r_value, &scope, span)?;

if tail.is_empty() {
match execute_body(body, &mut scope, out_values) {
match execute_body(body, &scope, out_values) {
Err(FunctionCarrier::Continue) => {}
Err(error) => return Err(error),
Ok(_value) => {}
}
} else {
execute_for_iterations(tail, body, out_values, &mut scope, span)?;
execute_for_iterations(tail, body, out_values, &scope, span)?;
}
}
}
Expand Down
Loading