Skip to content

Commit

Permalink
[BUGFIX release] Avoid re-freezing already frozen objects.
Browse files Browse the repository at this point in the history
When a helper is invoked we pass in the arguments, those arguments
are frozen (via Object.freeze) so that helpers can't mutate them
(and cause issues in the rendering engine itself). When a helper
doesn't have hash arguments, we use a shared `EMPTY_HASH` empty
object to avoid allocating a bunch of empty objects for no reason
(we do roughly the same thing when no positional params are
passed). Since these objects are shared they are being frozen
over and over again (throughout the lifetime of the running
application). Turns out that there is what we think is almost
certainly a bug in Chrome, where re-freezing the same object many
times starts taking significantly more time upon each freeze
attempt.

This change introduces a guard to ensure we do not re-freeze repeatedly.

Thanks to @krisselden for identifying the root cause.
  • Loading branch information
rwjblue committed May 31, 2017
1 parent 5291c74 commit 29f1898
Showing 1 changed file with 19 additions and 12 deletions.
31 changes: 19 additions & 12 deletions packages/ember-glimmer/lib/utils/references.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@ export const UPDATE = symbol('UPDATE');

export { NULL_REFERENCE, UNDEFINED_REFERENCE } from '@glimmer/runtime';

let maybeFreeze = () => {};
if (DEBUG) {
maybeFreeze = (obj) => {
// re-freezing an already frozen object introduces a significant
// performance penalty on Chrome (tested through 59).
//
// See: https://bugs.chromium.org/p/v8/issues/detail?id=6450
if (!Object.isFrozen(obj) && HAS_NATIVE_WEAKMAP) {
Object.freeze(obj);
}
}
}

// @abstract
// @implements PathReference
class EmberPathReference {
Expand Down Expand Up @@ -296,10 +309,8 @@ export class SimpleHelperReference extends CachedReference {
let namedValue = named.value();

if (DEBUG) {
if (HAS_NATIVE_WEAKMAP) {
Object.freeze(positionalValue);
Object.freeze(namedValue);
}
maybeFreeze(positionalValue);
maybeFreeze(namedValue);
}

let result = helper(positionalValue, namedValue);
Expand Down Expand Up @@ -333,10 +344,8 @@ export class SimpleHelperReference extends CachedReference {
let namedValue = named.value();

if (DEBUG) {
if (HAS_NATIVE_WEAKMAP) {
Object.freeze(positionalValue);
Object.freeze(namedValue);
}
maybeFreeze(positionalValue);
maybeFreeze(namedValue);
}

return helper(positionalValue, namedValue);
Expand Down Expand Up @@ -365,10 +374,8 @@ export class ClassBasedHelperReference extends CachedReference {
let namedValue = named.value();

if (DEBUG) {
if (HAS_NATIVE_WEAKMAP) {
Object.freeze(positionalValue);
Object.freeze(namedValue);
}
maybeFreeze(positionalValue);
maybeFreeze(namedValue);
}

return instance.compute(positionalValue, namedValue);
Expand Down

0 comments on commit 29f1898

Please sign in to comment.