Add stack size calculation to code object to pre-allocate value stack #1327
Conversation
|
This seems like it's going to add a bunch of code, for a possible speed up. Is it justified? |
|
@cthulahoops Looking for this one? |
vm/src/frame.rs
Outdated
| @@ -85,7 +85,7 @@ pub struct Frame { | |||
| stack: RefCell<Vec<PyObjectRef>>, // The main data frame of the stack machine | |||
| blocks: RefCell<Vec<Block>>, // Block frames, for controlling loops and exceptions | |||
| pub scope: Scope, // Variables | |||
| pub lasti: RefCell<usize>, // index of last instruction ran | |||
| lasti: RefCell<usize>, // index of last instruction ran | |||
There was a problem hiding this comment.
If you're going to touch this, do you want to make it a Cell<usize>? It really doesn't need to be a RefCell.
vm/src/frame.rs
Outdated
| @@ -1244,6 +1261,7 @@ impl Frame { | |||
|
|
|||
| pub fn push_value(&self, obj: PyObjectRef) { | |||
| self.stack.borrow_mut().push(obj); | |||
| // println!("Stack size: {}, capa: {}", self.stack.borrow().len(), self.stack.borrow().capacity()); | |||
There was a problem hiding this comment.
| // println!("Stack size: {}, capa: {}", self.stack.borrow().len(), self.stack.borrow().capacity()); |
| self.current_stack_size += effect; | ||
| if self.current_stack_size > self.max_stack { | ||
| self.max_stack = self.current_stack_size; | ||
| } |
There was a problem hiding this comment.
You could get rid of the properties on CodeObjectStream and just mutate max_stack_size on CodeObject:
let max_stack_size = &mut self.code.max_stack_size;
*max_stack_size = std::cmp::max(*max_stack_size, *max_stack_size + effect);There was a problem hiding this comment.
I understand. The thing is, I would like to gather all instructions in the stream, and in the end, create the immutable codeobject. This makes the code easier to understand in my opinion. That is the reason why I implemented the code like it is.
This change is not justified, but it is a copy of an idea from cpython and micropython. |
|
This has been implemented for about a year now iirc, I just never closed this PR. |
This is still early work. Comments / ideas are welcome!
Idea is to determine the worst case stack size, and prevent a re-allocation of the value stack. Minor performance win, a lot of work :).