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

Represent opcodes as integers in wire-format #392

Merged
merged 1 commit into from Jan 20, 2017

Conversation

Projects
None yet
7 participants
@chadhietala
Copy link
Member

chadhietala commented Jan 19, 2017

This introduces the idea of representing the wire-format opcodes as an integer instead of a string. Consider the following:

<div>Hello World</div>

Today pre-compiles to:

[
  ['open-element, 'div', []],
  ['flush-element'],
  ['text', 'Hello World'],
  ['close-element']
]

After this PR

[
  [11, 'div', []],
  [13],
  [0, 'Hello World'],
  [14]
]

In my experiments this will have a slight decrease in gzip size in a large application, however does has a decent size win post-unzipping and residing in memory.

Update

The second commit introduces an enum for the most common html tags. This would produce the following:

[
  [11, 0, []],
  [13],
  [0, 'Hello World'],
  [14]
]

@chadhietala chadhietala force-pushed the chadhietala:wire-format-opcodes branch from 376e2c5 to 743273d Jan 19, 2017

@@ -0,0 +1,60 @@
export enum Tags {

This comment has been minimized.

Copy link
@cibernox

cibernox Jan 19, 2017

Contributor

Would it worth to add svg tags here? I use those more often than param or abbr, by example.
Future idea: Is it possible to statically gather the used tags analysing the templates?

This comment has been minimized.

Copy link
@chadhietala

chadhietala Jan 19, 2017

Author Member

I can add SVG tags.Discovering is pretty difficult to do ahead of time specifically with the custom element stuff. For example we do not now AOT if <foo-bar /> is a custom element or if it's a glimmer component.

This comment has been minimized.

Copy link
@tomdale

tomdale Jan 19, 2017

Contributor

@chadhietala Isn't there a plan to provide the compiler with a list of known component names?

This comment has been minimized.

Copy link
@chadhietala

chadhietala Jan 19, 2017

Author Member

This is true, needs module unification.


// Expressions

Unkown,

This comment has been minimized.

Copy link
@dmfenton

@chadhietala chadhietala force-pushed the chadhietala:wire-format-opcodes branch from 4bffd49 to 91e4e7b Jan 19, 2017

@chadhietala

This comment has been minimized.

Copy link
Member Author

chadhietala commented Jan 19, 2017

Just tried this in one of our apps. Saved 14KB on GZIP size but 800KB post-inflation.

@jhartman

This comment has been minimized.

Copy link

jhartman commented Jan 19, 2017

Chad, what is the impact on forwards & backwards compatibility? What happens if there is a new integer the client doesn't understand?

@mixonic

This comment has been minimized.

Copy link
Contributor

mixonic commented Jan 19, 2017

@jhartman the Glimmer always requires the same version compiler and runtime on both sides of the wire format. This applies to Ember as well: You must use the Ember 2.9 template compiler with the Ember 2.9 runtime for example.

@topaxi

This comment has been minimized.

Copy link

topaxi commented Jan 20, 2017

Attributes might be interesting too? class, width, height, fill-rule etc. should be pretty common.

@chadhietala

This comment has been minimized.

Copy link
Member Author

chadhietala commented Jan 20, 2017

Running this against travis-web here are the app sizes:

Baseline:

Ember@2.10.0                                  910.02 KB (185.2 KB gzipped)

Commit 1:

Ember@master+glimmer#ce58fe8   807.38 KB (182.19 KB gzipped)

Commit 2:

Ember@master+glimmer#91e4e7b   804.41 KB (182.01 KB gzipped)

Commit 2 doesn't really provide as much value in its current state so I'm likely to just land the opcodes change. I have some ideas to further drop the size and deal with the repetition of user-land strings, but is out of scope for this set of changes.

@chadhietala chadhietala force-pushed the chadhietala:wire-format-opcodes branch from 91e4e7b to 39faaf2 Jan 20, 2017

@chadhietala chadhietala force-pushed the chadhietala:wire-format-opcodes branch from 39faaf2 to 68503e9 Jan 20, 2017

@chadhietala

This comment has been minimized.

Copy link
Member Author

chadhietala commented Jan 20, 2017

@krisselden +1 in slack

@chadhietala chadhietala merged commit 29f4e45 into glimmerjs:master Jan 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@chadhietala chadhietala deleted the chadhietala:wire-format-opcodes branch Jan 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.