Asset size regression in v3.17.0

[Turbo87]

We noticed in one of our apps that the Ember v3.17.0 release significantly increase our asset sizes:

Before

• dist/assets/app.js: 613.18 KB (78.46 KB gzipped)
• dist/assets/vendor.js: 2.34 MB (558.13 KB gzipped)

After

• dist/assets/app.js: 690.43 KB (89.22 KB gzipped)
• dist/assets/vendor.js: 2.42 MB (572.39 KB gzipped)

/cc @pzuraq @rwjblue

[makepanic]

Can confirm in a fresh new ember project:

2.16.0

• vendor: 692.58 KB (176.09 KB gzipped)
• app: 9.82 KB (2.15 KB gzipped)

2.17.0

• vendor: 715.12 KB (183.64 KB gzipped)
• app: 9.94 KB (2.2 KB gzipped)

Did some vendor diffs and it seems to be caused by some glimmer-vm updates, e.g. glimmerjs/glimmer-vm@4c56c21#diff-64c3cabffb164d9a2c4bf2d427094e19 via 699439c

[boris-petrov]

Same here.

2.16.3

• vendor: 886KB (213KB brotli)
• app: 1.01MB (126KB brotli)

2.17.0

• vendor: 907KB (219KB brotli)
• app: 1.15MB (137KB brotli)

Also, I noticed a ~5% performance degradation in our integration tests going from 2.16.3 to 2.17.0.

P.S. @Turbo87 - could it be because of the HBS minifier? I'm also using it. Could it have "broken" somehow for the new templates generated by the updated Glimmer VM?

[rwjblue]

@boris-petrov - Mind checking that for us? I think you should be able to remove the minifier from your apps package.json...

[krisselden]

Looks like this is coming from changes in the glimmer wireformat that is bloating the template sizes a bit. In particular doing a diff on the app is https://github.com/glimmerjs/glimmer-vm/blob/11cc83b2adf220bef298a1a6298c9e4e750c9fe1/packages/%40glimmer/interfaces/lib/compile/wire-format.d.ts#L76

[boris-petrov]

@rwjblue - I believe @krisselden is correct - this has nothing to do with ember-hbs-minifier. I tried a brand new Ember app with the following helper:

import Helper from '@ember/component/helper';
export default Helper.helper(() => true);

And two components:

other-component.hbs

{{some-component foo=1}}

some-component.hbs

{{some-helper 1 2 3 @foo}}

3.16.3 produced for the two components:

other-component.hbs:

block: "{\"symbols\":[],\"statements\":[[1,[28,\"some-component\",null,[[\"foo\"],[1]]],false],[0,\"\\n\"]],\"hasEval\":false}",

some-component.hbs:

block: "{\"symbols\":[\"@foo\"],\"statements\":[[1,[28,\"some-helper\",[1,2,3,[23,1,[]]],null],false],[0,\"\\n\"]],\"hasEval\":false}",

The same for 3.17:

other-component.hbs:

block: "{\"symbols\":[],\"statements\":[[1,0,0,0,[31,2,14,[27,[26,0,\"CallHead\"],[]],null,[[\"foo\"],[1]]]],[1,1,0,0,\"\\n\"]],\"hasEval\":false,\"upvars\":[\"some-component\"]}",

some-component.hbs:

block: "{\"symbols\":[\"@foo\"],\"statements\":[[1,0,0,0,[31,2,11,[27,[26,0,\"CallHead\"],[]],[1,2,3,[27,[24,1],[]]],null]],[1,1,0,0,\"\\n\"]],\"hasEval\":false,\"upvars\":[\"some-helper\"]}",

I wouldn't call that a "bug", it's just not very nice. The bigger the app you have, the bigger the templates will have gotten.

[acorncom]

The change in wire format seems to be related to the work done here: glimmerjs/glimmer-vm#972

[krisselden]

glimmerjs/glimmer-vm#1056

[krisselden]

Ember 3.16

- dist/assets/ember-observer.js: 706.89 KB (66.89 KB gzipped)

Ember 3.17

 - dist/assets/ember-observer.js: 743.7 KB (69.63 KB gzipped)

After const enum string to number

 - dist/assets/ember-observer.js: 739.58 KB (69.44 KB gzipped)

Removing loc data

 - dist/assets/ember-observer.js: 723.55 KB (68.06 KB gzipped)

Flatten Get + GetPath + GetContextualFree expressions

- dist/assets/ember-observer.js: 721.99 KB (68.04 KB gzipped)

[krisselden]

I made a util, if you wouldn't mind running on your apps to let me know how the patch does

https://www.npmjs.com/package/ember-template-size

[Turbo87]

$ npx ember-template-size .
npx: installed 11 in 1.766s
{
  '3.16.3': {
    version: '3.16.3',
    original: 793656,
    compiled: 1223295,
    brotli: 242598,
    gzip: 242606
  },
  '3.17.0-patched': {
    version: '3.17.0-patched',
    original: 793656,
    compiled: 1299096,
    brotli: 258803,
    gzip: 258826
  },
  '3.17.0': {
    version: '3.17.0',
    original: 793656,
    compiled: 1624741,
    brotli: 294760,
    gzip: 294780
  }
}

[Turbo87]

and for https://github.com/rust-lang/crates.io:

{
  '3.16.3': {
    version: '3.16.3',
    original: 73613,
    compiled: 143264,
    brotli: 32477,
    gzip: 32608
  },
  '3.17.0-patched': {
    version: '3.17.0-patched',
    original: 73613,
    compiled: 160526,
    brotli: 34663,
    gzip: 34803
  },
  '3.17.0': {
    version: '3.17.0',
    original: 73613,
    compiled: 183711,
    brotli: 36903,
    gzip: 37070
  }
}

[makepanic]

for us:

{
  '3.16.3': {
    version: '3.16.3',
    original: 281240,
    compiled: 450040,
    brotli: 98120,
    gzip: 98142
  },
  '3.17.0-patched': {
    version: '3.17.0-patched',
    original: 281240,
    compiled: 494042,
    brotli: 105152,
    gzip: 105178
  },
  '3.17.0': {
    version: '3.17.0',
    original: 281240,
    compiled: 596287,
    brotli: 117757,
    gzip: 117784
  }
}

[krisselden]

The patch represents most the low hanging fruit ideas I had. I have a couple more ideas, but based on the results so far, not going to recover the rest of the regression. The refactor that caused the regression was large and enables some future direction and I have limited time.

I have 2 more simple ideas, flatten [Append, 1 | 0, ...] into [TrustingAppend | Append, ..] and then there is wire format tuple that uses a boolean flag that can use 1 | 0 instead.