From d8f593c10017cd7ac2db220a7b1d8bbf7bb49d19 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 29 Oct 2015 21:59:01 -0700 Subject: [PATCH 1/4] perf(copy): use ES6 Map to track source/destination objects --- src/Angular.js | 58 ++++++++++++++++++++++++++++++++++++++------- test/.eslintrc.json | 2 ++ test/AngularSpec.js | 33 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 60463f1e1eb3..a4156c7f47b7 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -743,6 +743,49 @@ function arrayRemove(array, value) { return index; } + +function ES6MapShim() { + this._keys = []; + this._values = []; +} +ES6MapShim.prototype = { + get: function(key) { + var idx = this._keys.indexOf(key); + if (idx !== -1) { + return this._values[idx]; + } + }, + set: function(key, value) { + var idx = this._keys.indexOf(key); + if (idx === -1) { + idx = this._keys.length; + } + this._keys[idx] = key; + this._values[idx] = value; + return this; + } +}; + +function testES6Map(Map) { + var m, o = {}; + return isFunction(Map) && (m = new Map()) + + // Required functions + && isFunction(m.get) && isFunction(m.set) + + // Number keys, must not call toString + && m.get(1) === undefined + && m.set(1, o) === m + && m.get(1) === o + && m.get('1') === undefined + + // Object keys, must use instance as key and not call toString + && m.set(o, 2) === m && m.get(o) === 2 + && m.get({}) === undefined; +} + +var ES6Map = testES6Map(window.Map) ? window.Map : ES6MapShim; + /** * @ngdoc function * @name angular.copy @@ -809,8 +852,7 @@ function arrayRemove(array, value) { */ function copy(source, destination) { - var stackSource = []; - var stackDest = []; + var stack = new ES6Map(); if (destination) { if (isTypedArray(destination) || isArrayBuffer(destination)) { @@ -831,8 +873,7 @@ function copy(source, destination) { }); } - stackSource.push(source); - stackDest.push(destination); + stack.set(source, destination); return copyRecurse(source, destination); } @@ -876,9 +917,9 @@ function copy(source, destination) { } // Already copied values - var index = stackSource.indexOf(source); - if (index !== -1) { - return stackDest[index]; + var existingCopy = stack.get(source); + if (existingCopy) { + return existingCopy; } if (isWindow(source) || isScope(source)) { @@ -894,8 +935,7 @@ function copy(source, destination) { needsRecurse = true; } - stackSource.push(source); - stackDest.push(destination); + stack.set(source, destination); return needsRecurse ? copyRecurse(source, destination) diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 2bf89b2bb07e..195b10023d46 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -103,6 +103,8 @@ "getBlockNodes": false, "createMap": false, "VALIDITY_STATE_PROPERTY": true, + "testES6Map": true, + "ES6MapShim": true, /* AngularPublic.js */ "version": false, diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 6fa00567139a..4f05f567be76 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -25,6 +25,39 @@ describe('angular', function() { }); }); + describe('ES6Map', function() { + it('should test for bad Map objects', function() { + expect(testES6Map()).toBe(false); + expect(testES6Map(null)).toBe(false); + expect(testES6Map(3)).toBe(false); + expect(testES6Map({})).toBe(false); + }); + + it('should test for bad Map shims', function() { + function NoMethods() {} + + function ToStringOnKeys() { + this._vals = {}; + } + ToStringOnKeys.prototype = { + get: function(key) { + return this._vals[key]; + }, + set: function(key, val) { + this._vals[key] = val; + return this; + } + }; + + expect(testES6Map(NoMethods)).toBe(false); + expect(testES6Map(ToStringOnKeys)).toBe(false); + }); + + it('should pass the ES6Map test with ES6MapShim', function() { + expect(testES6Map(ES6MapShim)).toBe(true); + }); + }); + describe('copy', function() { it('should return same object', function() { var obj = {}; From ed16ce11f57e324a5b9cdc3bca6a577459f8b0c0 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 10 Dec 2015 23:20:07 -0800 Subject: [PATCH 2/4] fixup! perf(copy): Saving the last key/index lookup in ESMapShim --- src/Angular.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index a4156c7f47b7..b2db65588e42 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -747,18 +747,27 @@ function arrayRemove(array, value) { function ES6MapShim() { this._keys = []; this._values = []; + this._lastKey = NaN; + this._lastIndex = -1; } ES6MapShim.prototype = { + _idx: function(key) { + if (key === this._lastKey) { + return this._lastIndex; + } + return (this._lastIndex = (this._keys.indexOf(this._lastKey = key))); + }, get: function(key) { - var idx = this._keys.indexOf(key); + var idx = this._idx(key); if (idx !== -1) { return this._values[idx]; } }, set: function(key, value) { - var idx = this._keys.indexOf(key); + var idx = this._idx(key); if (idx === -1) { - idx = this._keys.length; + idx = this._lastIndex = this._keys.length; + this._lastKey = key; } this._keys[idx] = key; this._values[idx] = value; From 6a102ef45333cdcb86ec8cace5fc9584e617b4c9 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sat, 27 Aug 2016 11:25:01 -0700 Subject: [PATCH 3/4] fixup! perf(copy): replace Map feature-test with native-test --- src/Angular.js | 30 +++++++++--------------------- test/AngularSpec.js | 33 --------------------------------- 2 files changed, 9 insertions(+), 54 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index b2db65588e42..874c53c08b9d 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -743,7 +743,9 @@ function arrayRemove(array, value) { return index; } - +// A minimal ES6 Map implementation. +// Should be bug/feature equivelent to the native implementations of supported browsers. +// See https://kangax.github.io/compat-table/es6/#test-Map function ES6MapShim() { this._keys = []; this._values = []; @@ -771,29 +773,15 @@ ES6MapShim.prototype = { } this._keys[idx] = key; this._values[idx] = value; - return this; + + // Support: IE11 + // Do not `return this` to simulate the partial IE11 implementation } }; -function testES6Map(Map) { - var m, o = {}; - return isFunction(Map) && (m = new Map()) - - // Required functions - && isFunction(m.get) && isFunction(m.set) - - // Number keys, must not call toString - && m.get(1) === undefined - && m.set(1, o) === m - && m.get(1) === o - && m.get('1') === undefined - - // Object keys, must use instance as key and not call toString - && m.set(o, 2) === m && m.get(o) === 2 - && m.get({}) === undefined; -} - -var ES6Map = testES6Map(window.Map) ? window.Map : ES6MapShim; +var ES6Map = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' + ? window.Map + : ES6MapShim; /** * @ngdoc function diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 4f05f567be76..6fa00567139a 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -25,39 +25,6 @@ describe('angular', function() { }); }); - describe('ES6Map', function() { - it('should test for bad Map objects', function() { - expect(testES6Map()).toBe(false); - expect(testES6Map(null)).toBe(false); - expect(testES6Map(3)).toBe(false); - expect(testES6Map({})).toBe(false); - }); - - it('should test for bad Map shims', function() { - function NoMethods() {} - - function ToStringOnKeys() { - this._vals = {}; - } - ToStringOnKeys.prototype = { - get: function(key) { - return this._vals[key]; - }, - set: function(key, val) { - this._vals[key] = val; - return this; - } - }; - - expect(testES6Map(NoMethods)).toBe(false); - expect(testES6Map(ToStringOnKeys)).toBe(false); - }); - - it('should pass the ES6Map test with ES6MapShim', function() { - expect(testES6Map(ES6MapShim)).toBe(true); - }); - }); - describe('copy', function() { it('should return same object', function() { var obj = {}; From 5606fe88ea67166e795b6821b314b2ee74c9d9cb Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 28 Aug 2016 01:17:39 -0700 Subject: [PATCH 4/4] refactor(copy): avoid creating source/dest Map for simple copies --- src/Angular.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 874c53c08b9d..11595971104a 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -849,7 +849,7 @@ var ES6Map = isFunction(window.Map) && toString.call(window.Map.prototype) === ' */ function copy(source, destination) { - var stack = new ES6Map(); + var stack; if (destination) { if (isTypedArray(destination) || isArrayBuffer(destination)) { @@ -870,13 +870,14 @@ function copy(source, destination) { }); } - stack.set(source, destination); return copyRecurse(source, destination); } return copyElement(source); function copyRecurse(source, destination) { + (stack || (stack = new ES6Map())).set(source, destination); + var h = destination.$$hashKey; var key; if (isArray(source)) { @@ -914,7 +915,7 @@ function copy(source, destination) { } // Already copied values - var existingCopy = stack.get(source); + var existingCopy = stack && stack.get(source); if (existingCopy) { return existingCopy; } @@ -924,19 +925,15 @@ function copy(source, destination) { 'Can\'t copy! Making copies of Window or Scope instances is not supported.'); } - var needsRecurse = false; var destination = copyType(source); if (destination === undefined) { - destination = isArray(source) ? [] : Object.create(getPrototypeOf(source)); - needsRecurse = true; + destination = copyRecurse(source, isArray(source) ? [] : Object.create(getPrototypeOf(source))); + } else if (stack) { + stack.set(source, destination); } - stack.set(source, destination); - - return needsRecurse - ? copyRecurse(source, destination) - : destination; + return destination; } function copyType(source) {