From 567b1d75f7da75d86af20f3cfcda4ec20cc7aafe Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 31 May 2017 19:23:57 -0400 Subject: [PATCH] [BUGFIX release] Avoid re-freezing already frozen objects. 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 and reporting the upstream Chrome bug: https://bugs.chromium.org/p/v8/issues/detail?id=6450 --- .../ember-glimmer/lib/utils/references.js | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/packages/ember-glimmer/lib/utils/references.js b/packages/ember-glimmer/lib/utils/references.js index bb0d7a37f1a..75ac4b368e0 100644 --- a/packages/ember-glimmer/lib/utils/references.js +++ b/packages/ember-glimmer/lib/utils/references.js @@ -38,6 +38,22 @@ export const UPDATE = symbol('UPDATE'); export { NULL_REFERENCE, UNDEFINED_REFERENCE } from '@glimmer/runtime'; +let maybeFreeze; +if (DEBUG) { + // gaurding this in a DEBUG gaurd (as well as all invocations) + // so that it is properly stripped during the minification's + // dead code elimination + 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 { @@ -296,10 +312,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); @@ -333,10 +347,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); @@ -365,10 +377,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);