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

Start on ByteCode interpreter #854

Closed
wants to merge 3 commits into from
Closed

Start on ByteCode interpreter #854

wants to merge 3 commits into from

Conversation

jasonwilliams
Copy link
Member

@jasonwilliams jasonwilliams commented Oct 12, 2020

This does run!

Nodes implement CodeGen which generates instructions onto a stack held in Context.
The VM will interpret the instructions from Context.

There are some issues:

  • only basic instructions are added, but im working off https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/Bytecode for now it should be easy to add more in.
  • The Stack is a Vec, this isn't ideal (we may be able to live with it for now) but the stack should really be a fixed sized array. This isn't possible because Value can't be copied in there as it holds Rc and Gc values. Can we have fixed-sized Values that hold a pointer?
  • I needed to copy the Instruction vec, i couldn't find a way to pass in both ctx and ctx.instructions together without it moaning about "mutable borrowing" twice.
  • I need to change boa_cli back to the normal run.

As most of this is behind a feature flag it would be good to merge parts of it so we can work on as the main interpreter is active. Then one day we can switch over.

for

2 + 2

The instructions generated are

[boa/src/context.rs:689] &self.instruction_stack = [
    Int32       2,
    Int32       2,
    Add,
]
4

We can optimize even further here and just do the addition at CodeGen level and throw 4 onto the stack

Fixes #167
@Razican @HalidOdat

@jasonwilliams jasonwilliams mentioned this pull request Oct 12, 2020
@codecov
Copy link

codecov bot commented Oct 12, 2020

Codecov Report

Merging #854 into master will decrease coverage by 0.24%.
The diff coverage is 32.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #854      +/-   ##
==========================================
- Coverage   59.24%   58.99%   -0.25%     
==========================================
  Files         157      160       +3     
  Lines       10037    10132      +95     
==========================================
+ Hits         5946     5977      +31     
- Misses       4091     4155      +64     
Impacted Files Coverage Δ
boa/src/lib.rs 86.36% <ø> (ø)
boa/src/syntax/ast/node/operator/bin_op/mod.rs 66.08% <0.00%> (-4.29%) ⬇️
boa/src/syntax/ast/node/statement_list/mod.rs 42.22% <0.00%> (-6.50%) ⬇️
boa/src/vm/compilation.rs 0.00% <0.00%> (ø)
boa/src/vm/instructions.rs 0.00% <0.00%> (ø)
boa/src/vm/mod.rs 0.00% <0.00%> (ø)
boa_cli/src/main.rs 33.33% <ø> (ø)
boa/src/context.rs 57.38% <6.25%> (-3.71%) ⬇️
boa/src/builtins/object/mod.rs 64.86% <73.17%> (+3.18%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36b7c31...72b8680. Read the comment docs.

@HalidOdat HalidOdat added enhancement New feature or request performance Performance related changes and issues labels Oct 13, 2020
@jasonwilliams
Copy link
Member Author

continuing here #860

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance Performance related changes and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate Bytecode
2 participants