Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Compilation failures and unresolved reference-preservation, destructuring, and iterator-cleanup regressions block approval.
8 open issues
What changed in this PR
This PR replaces per-call VM allocation with one VM owned by each agent. Execution contexts track stack frames and executables, while bytecode explicitly preserves values and references across nested calls.
Changes:
- Introduces shared VM stacks, frame boundaries, and suspension/resumption handling.
- Updates bytecode generation and execution for shared registers and iterator cleanup.
- Moves executable ownership and garbage-collection tracing into execution contexts.
| File | Description |
|---|---|
nova_vm/src/engine/bytecode/vm/execute_instructions.rs |
Runs instructions against the agent’s shared VM. |
nova_vm/src/engine/bytecode/vm/binding_methods.rs |
Adapts destructuring to shared stacks and frame-relative slots. |
nova_vm/src/engine/bytecode/vm.rs |
Implements shared frames, suspension, and register handling. |
nova_vm/src/engine/bytecode/iterator.rs |
Accesses active iterators through the shared VM. |
nova_vm/src/engine/bytecode/instructions.rs |
Updates reference instructions and stack-slot addressing. |
nova_vm/src/engine/bytecode/executable.rs |
Exposes executable accessors and removes scoped wrappers. |
nova_vm/src/engine/bytecode/bytecode_compiler/for_in_of_statement.rs |
Revises async iteration and cleanup bytecode. |
nova_vm/src/engine/bytecode/bytecode_compiler/assignment.rs |
Preserves assignment references across nested execution. |
nova_vm/src/engine/bytecode/bytecode_compiler.rs |
Updates calls, optional chains, and delegated yields. |
nova_vm/src/ecmascript/types/spec/reference.rs |
Adds reference comparison and expression detection. |
nova_vm/src/ecmascript/types/language/object.rs |
Allows shape-based object creation through the heap. |
nova_vm/src/ecmascript/syntax_directed_operations/function_definitions.rs |
Executes function bodies through the agent. |
nova_vm/src/ecmascript/scripts_and_modules/script.rs |
Associates script executables with execution contexts. |
nova_vm/src/ecmascript/scripts_and_modules/module/module_semantics/source_text_module_records.rs |
Migrates module execution to the shared VM. |
nova_vm/src/ecmascript/execution/execution_context.rs |
Stores and traces executable and frame state. |
nova_vm/src/ecmascript/execution/agent.rs |
Owns the VM and coordinates execution-context transitions. |
nova_vm/src/ecmascript/builtins/global_object.rs |
Migrates eval execution to the agent. |
nova_vm/src/ecmascript/builtins/ecmascript_function.rs |
Initializes frame state for ordinary calls. |
nova_vm/src/ecmascript/builtins/control_abstraction_objects/generator_objects.rs |
Resumes generators through the agent. |
nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs |
Routes async-generator resumptions through the agent. |
nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects.rs |
Removes separate async-generator executable storage. |
nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_function_objects/await_reaction.rs |
Uses saved execution contexts when resuming awaits. |
nova_vm/src/ecmascript/builtins/builtin_function.rs |
Makes scoped argument handles cloneable. |
nova_vm/src/ecmascript/builtins/arguments.rs |
Accepts scoped argument handles by value. |
nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs |
Runs instance initializers through the shared VM. |
Review effort: Balanced
Comment on lines
+2022
to
+2024
| if ctx.is_call_optional_chain_this { | ||
| // Move kept reference into register. | ||
| ctx.add_instruction(Instruction::PopReference); |
Comment on lines
+528
to
+530
| agent.vm.ip = ip as usize; | ||
| agent.set_current_lexical_environment(lexical_environment); | ||
| agent.vm.result = Some(err.value().unbind()); |
| let mut idx = 0u32; | ||
| while let Some(result) = iterator | ||
| .step_value(agent, gc.reborrow()) | ||
| .unbind()? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stop allocating a VM per function call, instead run all functions in the same VM with a form of stack frames keeping the count to make sure we stay valid with that. The
resultandreferenceregisters are now actively clobbered: it's quite possible that there is a lot of places (odd JavaSccript, especially get/set methods throwing in the middle of member path evaluation or such, where the result is still invalidly clobbered, but at least the test262 tests have not regressed.