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

Move to Binary VM #376

Merged
merged 73 commits into from Jan 12, 2017

Conversation

@chadhietala
Copy link
Member

commented Jan 11, 2017

This moves away from mapping the wire-format into syntax objects which then compile into opcodes. For example we would do the following:

<div class="foo">Hello</div>

Precompiles to something like:

[
  ['open-primitive-element', ['div', ['class', 'foo']]],
  ['text', ['Hello']]
  ['close-element']
]

Which would generate a new instance of OpenPrimitiveElement, CloseElement, and Text syntax. This would be held in a list and then the each item in that list would compile into a LinkedList of opcodes. While this approach works, we ideally want to skip the Syntax objects completely and map the wire-format directly to opcodes.

This PR does several things:

  1. Removes the need for syntax objects at runtime
  2. Moves away from opcode objects in favor of builder functions
  3. Simplifies syntax refinement with the notion of macro builders which can do triage and set values on registers
  4. Opcodes are now true opcodes and represented as an enum
  5. At evaluation time these opcodes map into an evaluation function.

This likely addresses #356 as the opcodes are no longer held in a LinkedList but rather just ints in Uint32Array and map into functions.

@rwjblue

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

Is there a tracking Ember update branch?

@chadhietala

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2017

@chadhietala

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2017

I'm not actually sure what is going on this branch sense it looks like there are identical commits in the log. I'm wondering if this should just get squashed down. Also not very clear as to what is going on with the CI.

@chadhietala chadhietala force-pushed the runtime-null-checks branch from fe68106 to 74fe0ba Jan 11, 2017
wycats and others added 26 commits Dec 6, 2016
This is a simpler object model for use in testing and demos that can still
offer some guarantees around dirtiness (can be used for testing things
like the shouldComponentUpdate optimization).
The previous style is not compatible with node’s module resolution
algorithm, unfortunately, and baseUrl/paths hackery doesn’t help us
in CommonJS land.
This change largely eliminates implicit null
in glimmer-runtime with some WIP.
Remove unnecessary interface elements and extract the interface into the
interfaces package.
This is a simpler object model for use in testing and demos that can still
offer some guarantees around dirtiness (can be used for testing things
like the shouldComponentUpdate optimization).
The previous style is not compatible with node’s module resolution
algorithm, unfortunately, and baseUrl/paths hackery doesn’t help us
in CommonJS land.
This change largely eliminates implicit null
in glimmer-runtime with some WIP.
@chadhietala chadhietala force-pushed the runtime-null-checks branch to 54e414a Jan 11, 2017
@chadhietala chadhietala force-pushed the runtime-null-checks branch from 7fe3bb8 to fafdcc3 Jan 11, 2017
@chadhietala chadhietala force-pushed the runtime-null-checks branch from 6ae4c3b to bf7f6c0 Jan 11, 2017
@chadhietala chadhietala force-pushed the runtime-null-checks branch 2 times, most recently from 050c05b to d4009fe Jan 11, 2017
@chadhietala chadhietala force-pushed the runtime-null-checks branch from d4009fe to 0507336 Jan 11, 2017
@chadhietala chadhietala merged commit 1759c16 into master Jan 12, 2017
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@Turbo87

This comment has been minimized.

Copy link
Member

commented Jan 12, 2017

@chadhietala can you describe what this PR did? can't make sense of the diff... 🤔

@GavinJoyce

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2017

@chadhietala

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2017

@Turbo87 and @GavinJoyce I added a bit more context above.

@krisselden

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2017

@Turbo87 got rid of syntax objs, instead just directly works off the wire format to specialize and compile. Improves the VM loop execution speed by putting the opcodes into an array, instead of a list list off megamorphic object based opcodes, opcodes are now int mapped to functions and args, and paves the way for the upcoming stack based VM.

@krisselden

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2017

This is likely to have the bounds check issues with de-structuring within a loop over a jagged array. If the de-structuring from 2 types inlines and they have different lengths, the hoisting of their bounds check will cause problems.

@@ -182,6 +236,10 @@ export default class JavaScriptCompiler<T extends TemplateMeta> {
this.template.block.yields.add(to);
}

debugger() {
this.push(['debugger', null, null]);
}

This comment has been minimized.

Copy link
@chancancode

chancancode Jan 12, 2017

Contributor

@chadhietala what is the reason behind pushing this into the pre-compiler, vs just specializing it at runtime?

This comment has been minimized.

Copy link
@chadhietala

chadhietala Jan 16, 2017

Author Member

No specific reason. I probably should have had a better understanding of the system before implementing. Have to look at the specialization stuff.

@rwjblue rwjblue deleted the runtime-null-checks branch Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.