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

Backport template compiler improvements from 5.x #673

Merged
merged 6 commits into from
Mar 3, 2021

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Mar 3, 2021

  • Replace purgeModule cache busting with vm based sandboxing
  • Avoid building the template compiler cache key repeatedly
  • Remove usage of registerPlugin / unregisterPlugin
  • Ensure Ember 3.27+ can determine global for template compilation.
  • Ensure AST plugins have the same ordering as < [email protected].

References:

rwjblue and others added 5 commits March 3, 2021 16:13
The template compiler contents have to be evaluated separately for each
addon in the build pipeline. If they are **not** the AST plugins from
one addon leak through to other addons (or the app).

This issue led us to attempt to purge the normal node require cache (the
`purgeModule` code). This works (and has been in use for quite a while)
but causes a non-trivial amount of memory overhead since each of the addons'
ends up with a separate template compiler. This prevents JIT'ing and it
causes the source code of the template compiler itself to be in memory
many many many times (non-trivially increasing memory pressure).

Migrating to `vm.Script` and sandboxed contexts (similar to what is done
in FastBoot) resolves both of those issues. The script itself is cached
and not reevaluated each time (removing the memory pressure issues) and
the JIT information of the script context is also shared.

Thanks to @krisselden for pointing out this improvement!

(cherry picked from commit 8d5dbcf)
These APIs force Ember to use global mutable state (the list of plugins)
and require some pretty gnarly cache busting techniques to avoid having
addons break each other (due to the global mutable state leaking from
one addon to another).

In order to discourage this mutable state issue, Ember has deprecated
usage of `Ember.HTMLBars.registerPlugin` and
`Ember.HTMLBars.unregisterPlugin` (as of Ember 3.27).

This PR changes all invocations to pass the required AST transforms
directly in to the compiler invocation (instead of calling
`registerPlugin` before hand), and allows us to continue working
properly while avoiding the deprecation (and that evil mutable state).

(cherry picked from commit 1c813dc)
Node 12+ has access to `globalThis` (including within a VM context), but
older versions do not.

Due to the detection done in https://git.io/Jtb7s, when we can't find
`globalThis` (and don't define `global` global) evaluating
`ember-template-compiler.js` throws an error "unable to locate global
object".

This ensures that either `globalThis` or `global` are defined.

(cherry picked from commit 957dbc6)
We have to reverse these for reasons that are a bit bonkers. The initial
version of this system used `registeredPlugin` from
`ember-template-compiler.js` to set up these plugins (because Ember ~
1.13 only had `registerPlugin`, and there was no way to pass plugins
directly to the call to `compile`/`precompile`). Calling
`registerPlugin` unfortunately **inverted** the order of plugins (it
essentially did `PLUGINS = [plugin, ...PLUGINS]`).

Sooooooo...... we are forced to maintain that **absolutely bonkers**
ordering.

(cherry picked from commit d8e5dda)
@rwjblue rwjblue merged commit 653e0d2 into v4.x Mar 3, 2021
@rwjblue rwjblue deleted the backport-sandbox-changes branch March 3, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant