Repository navigation
Add stack size calculation to code object to pre-allocate value stack - #1327
windelbouwman wants to merge 5 commits into
Conversation
cthulahoops
left a comment
There was a problem hiding this comment.
Clicked approve in the wrong window, does commenting undo that?
|
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? |
coolreader18
left a comment
There was a problem hiding this comment.
I personally like this. CPython does it, it's more efficient, and it's really not all that much more code; it's essentially just a lookup table, both in the source code and the binary.
| 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.
|
|
||
| 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 :).