From 8134158739b36a2ce846b76e467515ed65038ca5 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 06:38:31 -0700 Subject: [PATCH 01/15] perf(Scope): change Scope#id to be a simple number In apps that create lots of scopes (apps with large tables) the uid generation shows up in the profiler and adds a few milliseconds. Using simple counter doesn't have this overhead. I think the initial fear of overflowing and thus using string alphanum sequence is unjustified because even if an app was to create lots of scopes non-stop, you could create about 28.6 million scopes per seconds for 10 years before you would reach a number that can't be accurately represented in JS BREAKING CHANGE: Scope#$id is now of time number rather than string. Since the id is primarily being used for debugging purposes this change should not affect anyone. --- src/Angular.js | 34 +++++++------------------ test/AngularSpec.js | 2 +- test/helpers/testabilityPatch.js | 2 +- test/ng/compileSpec.js | 42 +++++++++++++++---------------- test/ng/directive/ngRepeatSpec.js | 4 +-- 5 files changed, 34 insertions(+), 50 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index a6f8ef71fb9a..bb67d4c07803 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -164,7 +164,7 @@ var /** holds major version number for IE or NaN for real browsers */ angular = window.angular || (window.angular = {}), angularModule, nodeName_, - uid = ['0', '0', '0']; + uid = 0; /** * IE 11 changed the format of the UserAgent string. @@ -282,33 +282,17 @@ function reverseParams(iteratorFn) { } /** - * A consistent way of creating unique IDs in angular. The ID is a sequence of alpha numeric - * characters such as '012ABC'. The reason why we are not using simply a number counter is that - * the number string gets longer over time, and it can also overflow, where as the nextId - * will grow much slower, it is a string, and it will never overflow. + * A consistent way of creating unique IDs in angular. * - * @returns {string} an unique alpha-numeric string + * Using simple numbers allows us to generate 28.6 million unique ids per second for 10 years before + * we hit number precision issues in JavaScript. + * + * Math.pow(2,53) / 60 / 60 / 24 / 365 / 10 = 28.6M + * + * @returns {number} an unique alpha-numeric string */ function nextUid() { - var index = uid.length; - var digit; - - while(index) { - index--; - digit = uid[index].charCodeAt(0); - if (digit == 57 /*'9'*/) { - uid[index] = 'A'; - return uid.join(''); - } - if (digit == 90 /*'Z'*/) { - uid[index] = '0'; - } else { - uid[index] = String.fromCharCode(digit + 1); - return uid.join(''); - } - } - uid.unshift('0'); - return uid.join(''); + return ++uid; } diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 12142a918fc1..3709219859d5 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -925,7 +925,7 @@ describe('angular', function() { while(count--) { var current = nextUid(); - expect(current.match(/[\d\w]+/)).toBeTruthy(); + expect(typeof 2).toBe('number'); expect(seen[current]).toBeFalsy(); seen[current] = true; } diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js index 658d1883c102..2b9b71c72c4e 100644 --- a/test/helpers/testabilityPatch.js +++ b/test/helpers/testabilityPatch.js @@ -24,7 +24,7 @@ beforeEach(function() { } // This resets global id counter; - uid = ['0', '0', '0']; + uid = 0; // reset to jQuery or default to us. bindJQuery(); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index b5cb48ce9799..1c3d0d0293a0 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -1885,7 +1885,7 @@ describe('$compile', function() { it('should allow creation of new scopes', inject(function($rootScope, $compile, log) { element = $compile('
')($rootScope); - expect(log).toEqual('002; log-002-001; LOG'); + expect(log).toEqual('2; log-2-1; LOG'); expect(element.find('span').hasClass('ng-scope')).toBe(true); })); @@ -1893,7 +1893,7 @@ describe('$compile', function() { it('should allow creation of new isolated scopes for directives', inject( function($rootScope, $compile, log) { element = $compile('
')($rootScope); - expect(log).toEqual('log-001-no-parent; LOG; 002'); + expect(log).toEqual('log-1-no-parent; LOG; 2'); $rootScope.name = 'abc'; expect(iscope.$parent).toBe($rootScope); expect(iscope.name).toBeUndefined(); @@ -1905,11 +1905,11 @@ describe('$compile', function() { $httpBackend.expect('GET', 'tscope.html').respond('{{name}}; scopeId: {{$id}}'); element = $compile('
')($rootScope); $httpBackend.flush(); - expect(log).toEqual('log-002-001; LOG; 002'); + expect(log).toEqual('log-2-1; LOG; 2'); $rootScope.name = 'Jozo'; $rootScope.$apply(); - expect(element.text()).toBe('Jozo; scopeId: 002'); - expect(element.find('span').scope().$id).toBe('002'); + expect(element.text()).toBe('Jozo; scopeId: 2'); + expect(element.find('span').scope().$id).toBe(2); })); @@ -1919,11 +1919,11 @@ describe('$compile', function() { respond('

{{name}}; scopeId: {{$id}}

'); element = $compile('
')($rootScope); $httpBackend.flush(); - expect(log).toEqual('log-002-001; LOG; 002'); + expect(log).toEqual('log-2-1; LOG; 2'); $rootScope.name = 'Jozo'; $rootScope.$apply(); - expect(element.text()).toBe('Jozo; scopeId: 002'); - expect(element.find('a').scope().$id).toBe('002'); + expect(element.text()).toBe('Jozo; scopeId: 2'); + expect(element.find('a').scope().$id).toBe(2); })); @@ -1933,12 +1933,12 @@ describe('$compile', function() { respond('

{{name}}; scopeId: {{$id}} |

'); element = $compile('
')($rootScope); $httpBackend.flush(); - expect(log).toEqual('log-003-002; LOG; 003; log-005-004; LOG; 005; log-007-006; LOG; 007'); + expect(log).toEqual('log-3-2; LOG; 3; log-5-4; LOG; 5; log-7-6; LOG; 7'); $rootScope.name = 'Jozo'; $rootScope.$apply(); - expect(element.text()).toBe('Jozo; scopeId: 003 |Jozo; scopeId: 005 |Jozo; scopeId: 007 |'); - expect(element.find('p').scope().$id).toBe('003'); - expect(element.find('a').scope().$id).toBe('003'); + expect(element.text()).toBe('Jozo; scopeId: 3 |Jozo; scopeId: 5 |Jozo; scopeId: 7 |'); + expect(element.find('p').scope().$id).toBe(3); + expect(element.find('a').scope().$id).toBe(3); })); @@ -1947,7 +1947,7 @@ describe('$compile', function() { $httpBackend.expect('GET', 'tiscope.html').respond(''); element = $compile('
')($rootScope); $httpBackend.flush(); - expect(log).toEqual('log-002-001; LOG; 002'); + expect(log).toEqual('log-2-1; LOG; 2'); $rootScope.name = 'abc'; expect(iscope.$parent).toBe($rootScope); expect(iscope.name).toBeUndefined(); @@ -1967,7 +1967,7 @@ describe('$compile', function() { '' + '' )($rootScope); - expect(log).toEqual('002; 003; log-003-002; LOG; log-002-001; LOG; 004; log-004-001; LOG'); + expect(log).toEqual('2; 3; log-3-2; LOG; log-2-1; LOG; 4; log-4-1; LOG'); }) ); @@ -1976,7 +1976,7 @@ describe('$compile', function() { 'the scope', inject( function($rootScope, $compile, log) { element = $compile('
')($rootScope); - expect(log).toEqual('002; 002'); + expect(log).toEqual('2; 2'); }) ); @@ -1993,7 +1993,7 @@ describe('$compile', function() { it('should create new scope even at the root of the template', inject( function($rootScope, $compile, log) { element = $compile('
')($rootScope); - expect(log).toEqual('002'); + expect(log).toEqual('2'); }) ); @@ -2001,7 +2001,7 @@ describe('$compile', function() { it('should create isolate scope even at the root of the template', inject( function($rootScope, $compile, log) { element = $compile('
')($rootScope); - expect(log).toEqual('002'); + expect(log).toEqual('2'); }) ); @@ -3836,8 +3836,8 @@ describe('$compile', function() { element = $compile('
T:{{$parent.$id}}-{{$id}};
') ($rootScope); $rootScope.$apply(); - expect(element.text()).toEqual('W:001-002;T:001-003;'); - expect(jqLite(element.find('span')[0]).text()).toEqual('T:001-003'); + expect(element.text()).toEqual('W:1-2;T:1-3;'); + expect(jqLite(element.find('span')[0]).text()).toEqual('T:1-3'); expect(jqLite(element.find('span')[1]).text()).toEqual(';'); }); }); @@ -4175,7 +4175,7 @@ describe('$compile', function() { inject(function($compile) { element = $compile('
{{$id}}
')($rootScope); $rootScope.$apply(); - expect(element.text()).toBe($rootScope.$id); + expect(element.text()).toBe('' + $rootScope.$id); }); }); @@ -4235,7 +4235,7 @@ describe('$compile', function() { ($rootScope); $rootScope.$apply(); expect(log).toEqual('compile: ; link; LOG; LOG; HIGH'); - expect(element.text()).toEqual('001-002;001-003;'); + expect(element.text()).toEqual('1-2;1-3;'); }); }); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 0cc847e66634..79388f3a773b 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -976,7 +976,7 @@ describe('ngRepeat', function() { scope.items = [a, a, a]; scope.$digest(); expect($exceptionHandler.errors.shift().message). - toMatch(/^\[ngRepeat:dupes\] Duplicates in a repeater are not allowed\. Use 'track by' expression to specify unique keys\. Repeater: item in items, Duplicate key: object:003/); + toMatch(/^\[ngRepeat:dupes\] Duplicates in a repeater are not allowed\. Use 'track by' expression to specify unique keys\. Repeater: item in items, Duplicate key: object:3/); // recover scope.items = [a]; @@ -996,7 +996,7 @@ describe('ngRepeat', function() { scope.items = [d, d, d]; scope.$digest(); expect($exceptionHandler.errors.shift().message). - toMatch(/^\[ngRepeat:dupes\] Duplicates in a repeater are not allowed\. Use 'track by' expression to specify unique keys\. Repeater: item in items, Duplicate key: object:009/); + toMatch(/^\[ngRepeat:dupes\] Duplicates in a repeater are not allowed\. Use 'track by' expression to specify unique keys\. Repeater: item in items, Duplicate key: object:9/); // recover scope.items = [a]; From 8364e21dd1c50719bf26e893aa63cd971d43bfb6 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 07:16:15 -0700 Subject: [PATCH 02/15] perf(forEach): cache array length Micro-optimization :-) BREAKING CHANGE: forEach will iterate only over the initial number of items in the array. So if items are added to the array during the iteration, these won't be iterated over during the initial forEach call. This change also makes our forEach behave more like Array#forEach. --- src/Angular.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index bb67d4c07803..342eb0e1007f 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -226,8 +226,9 @@ function isArrayLike(obj) { * @param {Object=} context Object to become context (`this`) for the iterator function. * @returns {Object|Array} Reference to `obj`. */ + function forEach(obj, iterator, context) { - var key; + var key, length; if (obj) { if (isFunction(obj)) { for (key in obj) { @@ -240,8 +241,9 @@ function forEach(obj, iterator, context) { } else if (obj.forEach && obj.forEach !== forEach) { obj.forEach(iterator, context); } else if (isArrayLike(obj)) { - for (key = 0; key < obj.length; key++) + for (key = 0, length = obj.length; key < length; key++) { iterator.call(context, obj[key], key); + } } else { for (key in obj) { if (obj.hasOwnProperty(key)) { From 3d03e1996eaaf0e9f8f0cd5507ffafaa09573752 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 07:20:19 -0700 Subject: [PATCH 03/15] perf(shallowCopy): use Object.keys to improve performance This change is not IE8 friendly --- src/Angular.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 342eb0e1007f..705d235801b0 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -790,10 +790,12 @@ function copy(source, destination) { function shallowCopy(src, dst) { dst = dst || {}; - for(var key in src) { - // shallowCopy is only ever called by $compile nodeLinkFn, which has control over src - // so we don't need to worry about using our custom hasOwnProperty here - if (src.hasOwnProperty(key) && !(key.charAt(0) === '$' && key.charAt(1) === '$')) { + var keys = Object.keys(src); + + for (var i = 0, l = keys.length; i < l; i++) { + var key = keys[i]; + + if (!(key.charAt(0) === '$' && key.charAt(1) === '$')) { dst[key] = src[key]; } } From b9fbfb3583e99ef8d03cbb8bf0b996a30d19f114 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 08:16:04 -0700 Subject: [PATCH 04/15] perf(jqLite): don't use reflection to access expandoId Since we allow only one copy of Angular to be loaded at a time it doesn't make much sense randomly generate the expando property name and then be forced to use slow reflective calles to retrieve the IDs. --- src/jqLite.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 771c4312432d..78a32fc6b6d8 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -98,8 +98,9 @@ * @returns {Object} jQuery object. */ +JQLite.expando = 'ng'; + var jqCache = JQLite.cache = {}, - jqName = JQLite.expando = 'ng-' + new Date().getTime(), jqId = 1, addEventListenerFn = (window.document.addEventListener ? function(element, type, fn) {element.addEventListener(type, fn, false);} @@ -270,7 +271,7 @@ function jqLiteOff(element, type, fn, unsupported) { } function jqLiteRemoveData(element, name) { - var expandoId = element[jqName], + var expandoId = element.ng, expandoStore = jqCache[expandoId]; if (expandoStore) { @@ -284,17 +285,17 @@ function jqLiteRemoveData(element, name) { jqLiteOff(element); } delete jqCache[expandoId]; - element[jqName] = undefined; // ie does not allow deletion of attributes on elements. + element.ng = undefined; // don't delete DOM expandos. IE and Chrome don't like it } } function jqLiteExpandoStore(element, key, value) { - var expandoId = element[jqName], + var expandoId = element.ng, expandoStore = jqCache[expandoId || -1]; if (isDefined(value)) { if (!expandoStore) { - element[jqName] = expandoId = jqNextId(); + element.ng = expandoId = jqNextId(); expandoStore = jqCache[expandoId] = {}; } expandoStore[key] = value; From 1e18af4ca2a609643cc92db0a0590652e37d9f76 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 09:07:43 -0700 Subject: [PATCH 05/15] fix(jqLite): data should store data only on Element and Document nodes This is what jQuery does by default: https://github.com/jquery/jquery/blob/c18c6229c84cd2f0c9fe9f6fc3749e2c93608cc7/src/data/accepts.js#L16 We don't need to set data on text/comment nodes internally and if we don't allow setting data on these nodes, we don't need to worry about cleaning it up. BREAKING CHANGE: previously it was possible to set jqLite data on Text/Comment nodes, but now that is allowed only on Element and Document nodes just like in jQuery. We don't expect that app code actually depends on this accidental feature. --- src/jqLite.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/jqLite.js b/src/jqLite.js index 78a32fc6b6d8..4a32a3249ef7 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -315,7 +315,10 @@ function jqLiteData(element, key, value) { } if (isSetter) { - data[key] = value; + // set data only on Elements and Documents + if (element.nodeType === 1 || element.nodeType === 9) { + data[key] = value; + } } else { if (keyDefined) { if (isSimpleGetter) { From 6c2cb15b04c71e2076d5be070622f56f1a0686b0 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 08:23:37 -0700 Subject: [PATCH 06/15] perf(jqLite): optimize element dealocation Iterate only over elements and not nodes since we don't attach data or handlers to text/comment nodes. --- src/jqLite.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 4a32a3249ef7..5b09ab2ce096 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -240,8 +240,10 @@ function jqLiteClone(element) { function jqLiteDealoc(element){ jqLiteRemoveData(element); - for ( var i = 0, children = element.childNodes || []; i < children.length; i++) { - jqLiteDealoc(children[i]); + var childElement; + for ( var i = 0, children = element.children, l = (children && children.length) || 0; i < l; i++) { + childElement = children[i]; + jqLiteDealoc(childElement); } } From 9db0cdf16a8a4669407dff93df21711477520e95 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Wed, 4 Jun 2014 11:42:37 -0700 Subject: [PATCH 07/15] perf(jqLite): use classList for class manipulation if available All elements/browsers that don't have classList will still use the old setTimeout method. --- src/jqLite.js | 34 ++++++++++++++++++++++++---------- test/ngAnimate/animateSpec.js | 9 +++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 5b09ab2ce096..5846eb16249d 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -354,18 +354,32 @@ function jqLiteRemoveClass(element, cssClasses) { } function jqLiteAddClass(element, cssClasses) { - if (cssClasses && element.setAttribute) { - var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ') - .replace(/[\n\t]/g, " "); - - forEach(cssClasses.split(' '), function(cssClass) { - cssClass = trim(cssClass); - if (existingClasses.indexOf(' ' + cssClass + ' ') === -1) { - existingClasses += cssClass + ' '; + if (cssClasses) { + + // if browser and element support classList api use it + if (element.classList) { + cssClasses = trim(cssClasses); + if (cssClasses) { + cssClasses = cssClasses.split(/\s+/); + if (cssClasses.length === 1) { + element.classList.add(cssClasses[0]) + } else { + element.classList.add.apply(element.classList, cssClasses); + } } - }); + } else if (element.setAttribute) { + var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ') + .replace(/[\n\t]/g, " "); + + forEach(cssClasses.split(' '), function (cssClass) { + cssClass = trim(cssClass); + if (existingClasses.indexOf(' ' + cssClass + ' ') === -1) { + existingClasses += cssClass + ' '; + } + }); - element.setAttribute('class', trim(existingClasses)); + element.setAttribute('class', trim(existingClasses)); + } } } diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 89db1a12aae8..69e38729121d 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -3344,6 +3344,15 @@ describe("ngAnimate", function() { } node._setAttribute(prop, val); }; + node._classListAdd = node.classList.add; + node.classList.add = function(cssClass) { + if (cssClass === 'trigger-class') { + var propertyKey = ($sniffer.vendorPrefix == 'Webkit' ? '-webkit-' : '') + 'transition-property'; + capturedProperty = element.css(propertyKey); + } + + return node._classListAdd.apply(node.classList, arguments); + }; expect(capturedProperty).toBe('none'); $animate.addClass(element, 'trigger-class'); From bb5bc9642f558925b43d1171ab200b2ebe9ecfae Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 05:52:09 -0700 Subject: [PATCH 08/15] perf(jqLite): optimize adding nodes to a jqLite collection This code is very hot and in most cases we are wrapping just a single Node so we should optimize for that scenario. --- src/jqLite.js | 24 +++++++++++++++++++----- test/jqLiteSpec.js | 2 ++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 5846eb16249d..11e80348863d 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -383,17 +383,31 @@ function jqLiteAddClass(element, cssClasses) { } } + function jqLiteAddNodes(root, elements) { + // THIS CODE IS VERY HOT. Don't make changes without benchmarking. + if (elements) { - elements = (!elements.nodeName && isDefined(elements.length) && !isWindow(elements)) - ? elements - : [ elements ]; - for(var i=0; i < elements.length; i++) { - root.push(elements[i]); + + // if a Node (the most common case) + if (elements.nodeType) { + root[root.length++] = elements; + } else { + var length = elements.length; + + // if an Array or NodeList and not a Window + if (typeof length === 'number' && elements.window !== elements) { + if (length) { + push.apply(root, elements); + } + } else { + root[root.length++] = elements; + } } } } + function jqLiteController(element, name) { return jqLiteInheritedData(element, '$' + (name || 'ngController' ) + 'Controller'); } diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 8bab591ae173..feea87dac440 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -963,6 +963,8 @@ describe('jqLite', function() { }, detachEvent: noop }; + window.window = window; + var log; var jWindow = jqLite(window).on('hashchange', function() { log = 'works!'; From b52511cb08a4d063f455286e3b9f7aee264997ec Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 05:53:34 -0700 Subject: [PATCH 09/15] perf: optimize internal isWindow call Each window has a reference to itself, which is pretty unique so we can use that to simplify our isWindow check --- src/Angular.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Angular.js b/src/Angular.js index 705d235801b0..8eaf763428eb 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -536,7 +536,7 @@ function isRegExp(value) { * @returns {boolean} True if `obj` is a window obj. */ function isWindow(obj) { - return obj && obj.document && obj.location && obj.alert && obj.setInterval; + return obj && obj.window === obj; } From c027b838d6a3faaf5587572c340a5c3e08c18206 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 05:55:38 -0700 Subject: [PATCH 10/15] chore: name the event callback used by ngClick and friends This maskes looking at stack traces easier. Since we generate the callbacks for each event type at runtime and we can't set function's name because it's read-only, we have to use a generic name. --- src/ng/directive/ngEventDirs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ng/directive/ngEventDirs.js b/src/ng/directive/ngEventDirs.js index e69e6c8c61b0..185bc33bfc3d 100644 --- a/src/ng/directive/ngEventDirs.js +++ b/src/ng/directive/ngEventDirs.js @@ -45,7 +45,7 @@ forEach( return { compile: function($element, attr) { var fn = $parse(attr[directiveName]); - return function(scope, element, attr) { + return function ngEventHandler(scope, element) { element.on(lowercase(name), function(event) { scope.$apply(function() { fn(scope, {$event:event}); From aff9861662be3f8b5979545b46c37673d002372e Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 06:10:24 -0700 Subject: [PATCH 11/15] perf(jqLite): improve performance of jqLite#text This change is not compatible with IE8. --- src/jqLite.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 11e80348863d..a2dc1dc79ac5 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -594,23 +594,15 @@ forEach({ }, text: (function() { - var NODE_TYPE_TEXT_PROPERTY = []; - if (msie < 9) { - NODE_TYPE_TEXT_PROPERTY[1] = 'innerText'; /** Element **/ - NODE_TYPE_TEXT_PROPERTY[3] = 'nodeValue'; /** Text **/ - } else { - NODE_TYPE_TEXT_PROPERTY[1] = /** Element **/ - NODE_TYPE_TEXT_PROPERTY[3] = 'textContent'; /** Text **/ - } getText.$dv = ''; return getText; function getText(element, value) { - var textProp = NODE_TYPE_TEXT_PROPERTY[element.nodeType]; if (isUndefined(value)) { - return textProp ? element[textProp] : ''; + var nodeType = element.nodeType; + return (nodeType === 1 || nodeType === 3) ? element.textContent : ''; } - element[textProp] = value; + element.textContent = value; } })(), From 5465fa25ff3d2d4d8194981410df8c6b750190cd Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 06:10:46 -0700 Subject: [PATCH 12/15] test(jqLite): add a missing test for jqLite#text --- test/jqLiteSpec.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index feea87dac440..ccc5c33006f7 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -881,6 +881,12 @@ describe('jqLite', function() { expect(element.text('xyz') == element).toBeTruthy(); expect(element.text()).toEqual('xyzxyz'); }); + + it('shold return text only for element or text nodes', function() { + expect(jqLite('
foo
').text()).toBe('foo'); + expect(jqLite('
foo
').contents().eq(0).text()).toBe('foo'); + expect(jqLite(document.createComment('foo')).text()).toBe(''); + }); }); From df0b32bcae74a30071d21bea9e56355c24621aea Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 06:15:17 -0700 Subject: [PATCH 13/15] perf(jqLite): cache collection length for all methods that work on a single element This affects jqLite#html, #text, #attr, #prop, #css and others. --- src/jqLite.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index a2dc1dc79ac5..e584554fc789 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -639,6 +639,7 @@ forEach({ */ JQLite.prototype[name] = function(arg1, arg2) { var i, key; + var nodeCount = this.length; // jqLiteHasClass has only two arguments, but is a getter-only fn, so we need to special-case it // in a way that survives minification. @@ -648,7 +649,7 @@ forEach({ if (isObject(arg1)) { // we are a write, but the object properties are the key/values - for (i = 0; i < this.length; i++) { + for (i = 0; i < nodeCount; i++) { if (fn === jqLiteData) { // data() takes the whole object in jQuery fn(this[i], arg1); @@ -662,9 +663,10 @@ forEach({ return this; } else { // we are a read, so read the first child. + // TODO: do we still need this? var value = fn.$dv; // Only if we have $dv do we iterate over all, otherwise it is just the first element. - var jj = (value === undefined) ? Math.min(this.length, 1) : this.length; + var jj = (value === undefined) ? Math.min(nodeCount, 1) : nodeCount; for (var j = 0; j < jj; j++) { var nodeValue = fn(this[j], arg1, arg2); value = value ? value + nodeValue : nodeValue; @@ -673,7 +675,7 @@ forEach({ } } else { // we are a write, so apply to all children - for (i = 0; i < this.length; i++) { + for (i = 0; i < nodeCount; i++) { fn(this[i], arg1, arg2); } // return self for chaining From e4285e5914d351845995b64d4fe7045ba2fd0a05 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 06:36:20 -0700 Subject: [PATCH 14/15] perf(ngBind): set the ng-binding class during compilation instead of linking --- src/ng/directive/ngBind.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/ngBind.js b/src/ng/directive/ngBind.js index b37df64f2bba..b854ba75e09b 100644 --- a/src/ng/directive/ngBind.js +++ b/src/ng/directive/ngBind.js @@ -50,14 +50,19 @@ */ -var ngBindDirective = ngDirective(function(scope, element, attr) { - element.addClass('ng-binding').data('$binding', attr.ngBind); - scope.$watch(attr.ngBind, function ngBindWatchAction(value) { - // We are purposefully using == here rather than === because we want to - // catch when value is "null or undefined" - // jshint -W041 - element.text(value == undefined ? '' : value); - }); +var ngBindDirective = ngDirective({ + compile: function(templateElement) { + templateElement.addClass('ng-binding'); + return function (scope, element, attr) { + element.data('$binding', attr.ngBind); + scope.$watch(attr.ngBind, function ngBindWatchAction(value) { + // We are purposefully using == here rather than === because we want to + // catch when value is "null or undefined" + // jshint -W041 + element.text(value == undefined ? '' : value); + }); + }; + } }); From 5ffbcdf3a927eb7972806ad2272c7123461aaa72 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 5 Jun 2014 11:51:15 -0700 Subject: [PATCH 15/15] perf($compile): move ng-binding class stamping for interpolation into compile phase --- src/ng/compile.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index ce2002c80a73..75f82e7d0ff4 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1811,18 +1811,24 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (interpolateFn) { directives.push({ priority: 0, - compile: valueFn(function textInterpolateLinkFn(scope, node) { - var parent = node.parent(), - bindings = parent.data('$binding') || []; - // Need to interpolate again in case this is using one-time bindings in multiple clones - // of transcluded templates. - interpolateFn = $interpolate(text); - bindings.push(interpolateFn); - safeAddClass(parent.data('$binding', bindings), 'ng-binding'); - scope.$watch(interpolateFn, function interpolateFnWatchAction(value) { - node[0].nodeValue = value; - }); - }) + compile: function textInterpolateCompileFn(templateNode) { + safeAddClass(templateNode, 'ng-binding'); + + return function textInterpolateLinkFn(scope, node) { + + var parent = node.parent(), + bindings = parent.data('$binding') || []; + // Need to interpolate again in case this is using one-time bindings in multiple clones + // of transcluded templates. + interpolateFn = $interpolate(text); + //bindings.push(interpolateFn); + parent.data('$binding', bindings); + + scope.$watch(interpolateFn, function interpolateFnWatchAction(value) { + node[0].nodeValue = value; + }); + } + } }); } }