Permalink
Browse files

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.
1 parent 9971fbb commit 8c6a8171f9bdaa5cdabc0cc3f7d3ce10af7b434d @IgorMinar IgorMinar committed with rodyhaddad Jun 4, 2014
Showing with 34 additions and 50 deletions.
  1. +9 −25 src/Angular.js
  2. +1 −1 test/AngularSpec.js
  3. +1 −1 test/helpers/testabilityPatch.js
  4. +21 −21 test/ng/compileSpec.js
  5. +2 −2 test/ng/directive/ngRepeatSpec.js
View
@@ -167,7 +167,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.
@@ -285,33 +285,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;
}
View
@@ -958,7 +958,7 @@ describe('angular', function() {
while(count--) {
var current = nextUid();
- expect(current.match(/[\d\w]+/)).toBeTruthy();
+ expect(typeof current).toBe('number');
expect(seen[current]).toBeFalsy();
seen[current] = true;
}
@@ -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();
@@ -1885,15 +1885,15 @@ describe('$compile', function() {
it('should allow creation of new scopes', inject(function($rootScope, $compile, log) {
element = $compile('<div><span scope><a log></a></span></div>')($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);
}));
it('should allow creation of new isolated scopes for directives', inject(
function($rootScope, $compile, log) {
element = $compile('<div><span iscope><a log></a></span></div>')($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('<a log>{{name}}; scopeId: {{$id}}</a>');
element = $compile('<div><span tscope></span></div>')($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('<p><a log>{{name}}; scopeId: {{$id}}</a></p>');
element = $compile('<div><span trscope></span></div>')($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('<p><a log>{{name}}; scopeId: {{$id}} |</a></p>');
element = $compile('<div><span ng-repeat="i in [1,2,3]" trscope></span></div>')($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('<a log></a>');
element = $compile('<div><span tiscope></span></div>')($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() {
'</b>' +
'</div>'
)($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('<div class="scope-a; scope-b"></div>')($rootScope);
- expect(log).toEqual('002; 002');
+ expect(log).toEqual('2; 2');
})
);
@@ -2012,15 +2012,15 @@ describe('$compile', function() {
it('should create new scope even at the root of the template', inject(
function($rootScope, $compile, log) {
element = $compile('<div scope-a></div>')($rootScope);
- expect(log).toEqual('002');
+ expect(log).toEqual('2');
})
);
it('should create isolate scope even at the root of the template', inject(
function($rootScope, $compile, log) {
element = $compile('<div iscope></div>')($rootScope);
- expect(log).toEqual('002');
+ expect(log).toEqual('2');
})
);
@@ -3871,8 +3871,8 @@ describe('$compile', function() {
element = $compile('<div><div trans>T:{{$parent.$id}}-{{$id}}<span>;</span></div></div>')
($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(';');
});
});
@@ -4308,7 +4308,7 @@ describe('$compile', function() {
inject(function($compile) {
element = $compile('<div transclude>{{$id}}</div>')($rootScope);
$rootScope.$apply();
- expect(element.text()).toBe($rootScope.$id);
+ expect(element.text()).toBe('' + $rootScope.$id);
});
});
@@ -4544,7 +4544,7 @@ describe('$compile', function() {
($rootScope);
$rootScope.$apply();
expect(log).toEqual('compile: <!-- trans: text -->; link; LOG; LOG; HIGH');
- expect(element.text()).toEqual('001-002;001-003;');
+ expect(element.text()).toEqual('1-2;1-3;');
});
});
@@ -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];

0 comments on commit 8c6a817

Please sign in to comment.