Skip to content
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.

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

Refactor Context::run() method #3179

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Refactor Context::run() method #3179

merged 2 commits into from
Jul 31, 2023

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented Jul 27, 2023

Depends on #3059

This simplifies Context::run() and JsObject::call_internal() by taking some logic that is in specific types of functions (async generator or async function) and putting it in separate opcodes. This also has a performance benefit, since we only do those checks if we are using those features.

This PR is related to refactoring calling in VM.

It changes the following:

  • Adds opcodes: Generator, AsyncGeneratorClose, CreatePromiseCapability and CompletePromiseCapability
  • Makes Context::run() a simple loop that runs the opcodes, moves some logic to separate opcodes (resolving and rejecting promise capability and async generator close to separate opcode)
  • Moves JsObject::call_internal() logic to separate opcodes (generator creation and returning promise)
  • Improves trace of stack, displays the full stack per instruction execution, instead of just top of stack.
>> (++i) + (i++)

------------------------------------ Call Frame -- <main> ------------------------------------
Time          Opcode                     Operands                   Stack

7μs           GetName                    0000: 'i'                  [ 0 ]
1μs           Inc                                                   [ 1 ]
0μs           Dup                                                   [ 1, 1 ]
3μs           SetName                    0000: 'i'                  [ 1 ]
1μs           GetName                    0000: 'i'                  [ 1, 1 ]
0μs           IncPost                                               [ 1, 2, 1 ]
0μs           Swap                                                  [ 2, 1, 1 ]
1μs           SetName                    0000: 'i'                  [ 1, 1 ]
1μs           Add                                                   [ 2 ]
0μs           SetReturnValue                                        [ ]
0μs           Return                                                [ ]

@HalidOdat HalidOdat added technical debt execution Issues or PRs related to code execution labels Jul 27, 2023
@HalidOdat HalidOdat added this to the v0.18.0 milestone Jul 27, 2023
@HalidOdat HalidOdat requested a review from a team July 27, 2023 17:45
Base automatically changed from refactor-opcodes-environments to main July 29, 2023 21:52
@github-actions
Copy link

github-actions bot commented Jul 30, 2023

Test262 conformance changes

Test result main count PR count difference
Total 95,260 95,260 0
Passed 74,984 74,984 0
Ignored 19,198 19,198 0
Failed 1,078 1,078 0
Panics 0 0 0
Conformance 78.72% 78.72% 0.00%

@codecov
Copy link

codecov bot commented Jul 30, 2023

Codecov Report

Merging #3179 (c65e2de) into main (2b01ef1) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 51.45%.

@@            Coverage Diff             @@
##             main    #3179      +/-   ##
==========================================
- Coverage   50.41%   50.40%   -0.01%     
==========================================
  Files         436      436              
  Lines       42343    42375      +32     
==========================================
+ Hits        21346    21358      +12     
- Misses      20997    21017      +20     
Files Changed Coverage Δ
boa_engine/src/builtins/function/mod.rs 39.54% <0.00%> (-0.35%) ⬇️
boa_engine/src/module/source.rs 0.00% <0.00%> (ø)
boa_engine/src/vm/flowgraph/mod.rs 0.00% <0.00%> (ø)
boa_engine/src/vm/opcode/control_flow/jump.rs 92.68% <ø> (ø)
boa_engine/src/vm/opcode/mod.rs 69.23% <ø> (ø)
boa_engine/src/vm/mod.rs 60.97% <31.42%> (+7.55%) ⬆️
boa_engine/src/vm/code_block.rs 55.66% <57.14%> (-1.97%) ⬇️
boa_engine/src/vm/opcode/generator/mod.rs 35.06% <59.15%> (+20.60%) ⬆️
boa_engine/src/vm/opcode/await/mod.rs 71.62% <73.91%> (ø)
boa_engine/src/bytecompiler/declarations.rs 51.64% <100.00%> (ø)
... and 4 more

... and 3 files with indirect coverage changes

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Nice changes. I just found a few typos.

boa_engine/src/vm/opcode/generator/mod.rs Outdated Show resolved Hide resolved
boa_engine/src/vm/opcode/generator/mod.rs Outdated Show resolved Hide resolved
boa_engine/src/vm/opcode/mod.rs Outdated Show resolved Hide resolved
boa_engine/src/environments/runtime/mod.rs Outdated Show resolved Hide resolved
- Remove async generator close from run
- Remove promise capability from run
- Remove promise capability check from call_internal
- Remove generator creation from call_internal
- Move promise capability creation to separate opcode
@HalidOdat HalidOdat requested a review from a team July 30, 2023 17:55
Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Hey! This looks really great (especially with that other PR re calling the function with the stack🔥).

I just had one question but it could just be a stylistic/approach difference at it's core. Still figure I'd ask though 😄.

boa_engine/src/vm/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Nice work on this!

@HalidOdat HalidOdat enabled auto-merge July 31, 2023 00:52
@HalidOdat HalidOdat added this pull request to the merge queue Jul 31, 2023
Merged via the queue into main with commit d8bf5f5 Jul 31, 2023
9 checks passed
@HalidOdat HalidOdat deleted the refactor-run-method branch July 31, 2023 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execution Issues or PRs related to code execution technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants