Skip to content

Commit b8a3a38

Browse files
coolreader18coolreader18
committed
Manage the frame separation with 'ExecutingFrame'
1 parent 2368eae commit b8a3a38

3 files changed

Lines changed: 71 additions & 65 deletions

File tree

‎vm/src/frame.rs‎

Lines changed: 66 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ enum UnwindReason {
7777
}
7878

7979
struct FrameState {
80-
code: PyCodeRef,
81-
/// Variables
82-
scope: Scope,
8380
// We need 1 stack per frame
8481
/// The main data frame of the stack machine
8582
stack: Vec<PyObjectRef>,
@@ -161,65 +158,79 @@ impl Frame {
161158
// locals.extend(callargs);
162159

163160
Frame {
164-
// we have 2 references to the same code object (and scope) so that we don't have to
165-
// a) lock the state to get the code, which is immutable anyway
166-
// b) pass around a bunch of reference to code inside of the FrameState methods
167-
code: code.clone(),
168-
scope: scope.clone(),
161+
code,
162+
scope,
169163
state: Mutex::new(FrameState {
170-
code,
171-
scope,
172164
stack: Vec::new(),
173165
blocks: Vec::new(),
174166
lasti: 0,
175167
}),
176168
}
177169
}
170+
}
171+
172+
impl FrameRef {
173+
fn with_exec<R>(&self, f: impl FnOnce(ExecutingFrame) -> R) -> R {
174+
let mut state = self.state.lock().unwrap();
175+
let exec = ExecutingFrame {
176+
code: &self.code,
177+
scope: &self.scope,
178+
object: &self,
179+
state: &mut state,
180+
};
181+
f(exec)
182+
}
178183

179184
// #[cfg_attr(feature = "flame-it", flame("Frame"))]
180-
pub fn run(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<ExecutionResult> {
181-
zelf.state.lock().unwrap().run(&zelf, vm)
185+
pub fn run(&self, vm: &VirtualMachine) -> PyResult<ExecutionResult> {
186+
self.with_exec(|mut exec| exec.run(vm))
182187
}
183188

184189
pub(crate) fn resume(
185-
zelf: PyRef<Self>,
190+
&self,
186191
value: PyObjectRef,
187192
vm: &VirtualMachine,
188193
) -> PyResult<ExecutionResult> {
189-
let mut state = zelf.state.lock().unwrap();
190-
state.push_value(value);
191-
state.run(&zelf, vm)
194+
self.with_exec(|mut exec| {
195+
exec.push_value(value);
196+
exec.run(vm)
197+
})
192198
}
193199

194200
pub(crate) fn gen_throw(
195-
zelf: PyRef<Self>,
201+
&self,
196202
vm: &VirtualMachine,
197203
exc_type: PyObjectRef,
198204
exc_val: PyObjectRef,
199205
exc_tb: PyObjectRef,
200206
) -> PyResult<ExecutionResult> {
201-
zelf.state
202-
.lock()
203-
.unwrap()
204-
.gen_throw(&zelf, vm, exc_type, exc_val, exc_tb)
207+
self.with_exec(|mut exec| exec.gen_throw(vm, exc_type, exc_val, exc_tb))
205208
}
206209

207210
pub fn current_location(&self) -> bytecode::Location {
208-
let state = self.state.lock().unwrap();
209-
state.current_location()
211+
self.with_exec(|exec| exec.current_location())
210212
}
211213

212214
pub fn yield_from_target(&self) -> Option<PyObjectRef> {
213-
self.state.lock().unwrap().yield_from_target()
215+
self.with_exec(|exec| exec.yield_from_target())
214216
}
215217

216218
pub fn lasti(&self) -> usize {
217219
self.state.lock().unwrap().lasti
218220
}
219221
}
220222

221-
impl FrameState {
222-
fn run(&mut self, frameref: &FrameRef, vm: &VirtualMachine) -> PyResult<ExecutionResult> {
223+
/// An executing frame; essentially just a struct to combine the immutable data outside the mutex
224+
/// with the mutable data inside
225+
struct ExecutingFrame<'a> {
226+
code: &'a PyCodeRef,
227+
scope: &'a Scope,
228+
object: &'a FrameRef,
229+
state: &'a mut FrameState,
230+
}
231+
232+
impl ExecutingFrame<'_> {
233+
fn run(&mut self, vm: &VirtualMachine) -> PyResult<ExecutionResult> {
223234
flame_guard!(format!("Frame::run({})", self.code.obj_name));
224235
// Execute until return or exception:
225236
loop {
@@ -239,7 +250,7 @@ impl FrameState {
239250
let next = exception.traceback();
240251

241252
let new_traceback =
242-
PyTraceback::new(next, frameref.clone(), self.lasti, loc.row());
253+
PyTraceback::new(next, self.object.clone(), self.state.lasti, loc.row());
243254
exception.set_traceback(Some(new_traceback.into_ref(vm)));
244255
vm_trace!("Adding to traceback: {:?} {:?}", new_traceback, lineno);
245256

@@ -260,7 +271,8 @@ impl FrameState {
260271
}
261272

262273
fn yield_from_target(&self) -> Option<PyObjectRef> {
263-
if let Some(bytecode::Instruction::YieldFrom) = self.code.instructions.get(self.lasti) {
274+
if let Some(bytecode::Instruction::YieldFrom) = self.code.instructions.get(self.state.lasti)
275+
{
264276
Some(self.last_value())
265277
} else {
266278
None
@@ -269,7 +281,6 @@ impl FrameState {
269281

270282
fn gen_throw(
271283
&mut self,
272-
frameref: &FrameRef,
273284
vm: &VirtualMachine,
274285
exc_type: PyObjectRef,
275286
exc_val: PyObjectRef,
@@ -282,15 +293,15 @@ impl FrameState {
282293
};
283294
res.or_else(|err| {
284295
self.pop_value();
285-
self.lasti += 1;
296+
self.state.lasti += 1;
286297
let val = objiter::stop_iter_value(vm, &err)?;
287298
self._send(coro, val, vm)
288299
})
289300
.map(ExecutionResult::Yield)
290301
} else {
291302
let exception = exceptions::normalize(exc_type, exc_val, exc_tb, vm)?;
292303
match self.unwind_blocks(vm, UnwindReason::Raising { exception }) {
293-
Ok(None) => self.run(frameref, vm),
304+
Ok(None) => self.run(vm),
294305
Ok(Some(result)) => Ok(result),
295306
Err(exception) => Err(exception),
296307
}
@@ -301,11 +312,8 @@ impl FrameState {
301312
fn execute_instruction(&mut self, vm: &VirtualMachine) -> FrameResult {
302313
vm.check_signals()?;
303314

304-
// we get a separate code reference because we know that code isn't mutable, but borrowck will still
305-
// complain that we borrow immutable here and mutably later
306-
let co = self.code.clone();
307-
let instruction = &co.instructions[self.lasti];
308-
self.lasti += 1;
315+
let instruction = &self.code.instructions[self.state.lasti];
316+
self.state.lasti += 1;
309317

310318
flame_guard!(format!("Frame::execute_instruction({:?})", instruction));
311319

@@ -689,8 +697,8 @@ impl FrameState {
689697
}
690698
}
691699
bytecode::Instruction::Reverse { amount } => {
692-
let stack_len = self.stack.len();
693-
self.stack[stack_len - amount..stack_len].reverse();
700+
let stack_len = self.state.stack.len();
701+
self.state.stack[stack_len - amount..stack_len].reverse();
694702
Ok(None)
695703
}
696704
}
@@ -1108,7 +1116,7 @@ impl FrameState {
11081116
match result {
11091117
ExecutionResult::Yield(value) => {
11101118
// Set back program counter:
1111-
self.lasti -= 1;
1119+
self.state.lasti -= 1;
11121120
Ok(Some(ExecutionResult::Yield(value)))
11131121
}
11141122
ExecutionResult::Return(value) => {
@@ -1158,8 +1166,8 @@ impl FrameState {
11581166
fn jump(&mut self, label: bytecode::Label) {
11591167
let target_pc = self.code.label_map[&label];
11601168
#[cfg(feature = "vm-tracing-logging")]
1161-
trace!("jump from {:?} to {:?}", self.lasti, target_pc);
1162-
self.lasti = target_pc;
1169+
trace!("jump from {:?} to {:?}", self.state.lasti, target_pc);
1170+
self.state.lasti = target_pc;
11631171
}
11641172

11651173
/// The top of stack contains the iterator, lets push it forward
@@ -1411,47 +1419,51 @@ impl FrameState {
14111419
}
14121420

14131421
fn current_location(&self) -> bytecode::Location {
1414-
self.code.locations[self.lasti]
1422+
self.code.locations[self.state.lasti]
14151423
}
14161424

14171425
fn push_block(&mut self, typ: BlockType) {
1418-
self.blocks.push(Block {
1426+
self.state.blocks.push(Block {
14191427
typ,
1420-
level: self.stack.len(),
1428+
level: self.state.stack.len(),
14211429
});
14221430
}
14231431

14241432
fn pop_block(&mut self) -> Block {
1425-
let block = self.blocks.pop().expect("No more blocks to pop!");
1426-
self.stack.truncate(block.level);
1433+
let block = self.state.blocks.pop().expect("No more blocks to pop!");
1434+
self.state.stack.truncate(block.level);
14271435
block
14281436
}
14291437

14301438
fn current_block(&self) -> Option<Block> {
1431-
self.blocks.last().cloned()
1439+
self.state.blocks.last().cloned()
14321440
}
14331441

14341442
fn push_value(&mut self, obj: PyObjectRef) {
1435-
self.stack.push(obj);
1443+
self.state.stack.push(obj);
14361444
}
14371445

14381446
fn pop_value(&mut self) -> PyObjectRef {
1439-
self.stack
1447+
self.state
1448+
.stack
14401449
.pop()
14411450
.expect("Tried to pop value but there was nothing on the stack")
14421451
}
14431452

14441453
fn pop_multiple(&mut self, count: usize) -> Vec<PyObjectRef> {
1445-
let stack_len = self.stack.len();
1446-
self.stack.drain(stack_len - count..stack_len).collect()
1454+
let stack_len = self.state.stack.len();
1455+
self.state
1456+
.stack
1457+
.drain(stack_len - count..stack_len)
1458+
.collect()
14471459
}
14481460

14491461
fn last_value(&self) -> PyObjectRef {
1450-
self.stack.last().unwrap().clone()
1462+
self.state.stack.last().unwrap().clone()
14511463
}
14521464

14531465
fn nth_value(&self, depth: usize) -> PyObjectRef {
1454-
self.stack[self.stack.len() - depth - 1].clone()
1466+
self.state.stack[self.state.stack.len() - depth - 1].clone()
14551467
}
14561468
}
14571469

@@ -1477,7 +1489,7 @@ impl fmt::Debug for Frame {
14771489
.iter()
14781490
.map(|elem| format!("\n > {:?}", elem))
14791491
.collect::<String>();
1480-
let dict = state.scope.get_locals();
1492+
let dict = self.scope.get_locals();
14811493
let local_str = dict
14821494
.into_iter()
14831495
.map(|elem| format!("\n {:?} = {:?}", elem.0, elem.1))

‎vm/src/obj/objcoroinner.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::objtype::{self, PyClassRef};
22
use crate::exceptions::{self, PyBaseExceptionRef};
3-
use crate::frame::{ExecutionResult, Frame, FrameRef};
3+
use crate::frame::{ExecutionResult, FrameRef};
44
use crate::pyobject::{PyObjectRef, PyResult};
55
use crate::vm::VirtualMachine;
66

@@ -90,7 +90,7 @@ impl Coro {
9090
self.variant.name()
9191
)));
9292
}
93-
let result = self.run_with_context(vm, |f| Frame::resume(f, value, vm));
93+
let result = self.run_with_context(vm, |f| f.resume(value, vm));
9494
self.maybe_close(&result);
9595
match result {
9696
Ok(exec_res) => self.variant.exec_result(exec_res, vm),
@@ -123,29 +123,23 @@ impl Coro {
123123
if self.closed.get() {
124124
return Err(exceptions::normalize(exc_type, exc_val, exc_tb, vm)?);
125125
}
126-
vm.frames.borrow_mut().push(self.frame.clone());
127-
let result =
128-
self.run_with_context(vm, |f| Frame::gen_throw(f, vm, exc_type, exc_val, exc_tb));
126+
let result = self.run_with_context(vm, |f| f.gen_throw(vm, exc_type, exc_val, exc_tb));
129127
self.maybe_close(&result);
130-
vm.frames.borrow_mut().pop();
131128
self.variant.exec_result(result?, vm)
132129
}
133130

134131
pub fn close(&self, vm: &VirtualMachine) -> PyResult<()> {
135132
if self.closed.get() {
136133
return Ok(());
137134
}
138-
vm.frames.borrow_mut().push(self.frame.clone());
139135
let result = self.run_with_context(vm, |f| {
140-
Frame::gen_throw(
141-
f,
136+
f.gen_throw(
142137
vm,
143138
vm.ctx.exceptions.generator_exit.clone().into_object(),
144139
vm.get_none(),
145140
vm.get_none(),
146141
)
147142
});
148-
vm.frames.borrow_mut().pop();
149143
self.closed.set(true);
150144
match result {
151145
Ok(ExecutionResult::Yield(_)) => {

‎vm/src/vm.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ impl VirtualMachine {
299299
}
300300

301301
pub fn run_frame(&self, frame: FrameRef) -> PyResult<ExecutionResult> {
302-
self.with_frame(frame, |f| Frame::run(f, self))
302+
self.with_frame(frame, |f| f.run(self))
303303
}
304304

305305
fn check_recursive_call(&self, _where: &str) -> PyResult<()> {

0 commit comments

Comments
 (0)