Skip to content
This repository has been archived by the owner on Dec 6, 2017. It is now read-only.

Commit

Permalink
perf(Key): don't use Map.putIfAbsent -- too slow
Browse files Browse the repository at this point in the history
This change gives a 25% boost to Module benchmark.
  • Loading branch information
pavelgj committed May 5, 2014
1 parent f673267 commit 0930b37
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
16 changes: 12 additions & 4 deletions lib/key.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,19 @@ class Key {
* a previous call had. E.g. `identical(new Key(t, a), new Key(t, a))` holds.
*/
factory Key(Type type, [Type annotation]) {
var annotationToKey = _typeToAnnotationToKey.putIfAbsent(type, () => {});
return annotationToKey.putIfAbsent(
annotation, () => new Key._(type, annotation, _numInstances++));
// Don't use Map.putIfAbsent -- too slow!
var annotationToKey = _typeToAnnotationToKey[type];
if (annotationToKey == null) {
_typeToAnnotationToKey[type] = annotationToKey = {};
}
Key key = annotationToKey[annotation];
if (key == null) {

This comment has been minimized.

Copy link
@vicb

vicb May 5, 2014

Contributor

may be this can be further optimized as you know that the key will be null if you took the previous if branch ?

annotationToKey[annotation] =
key = new Key._(type, annotation, _numInstances++);
}
return key;
}

Key._(this.type, this.annotation, this.id);

String toString() {
Expand Down
5 changes: 5 additions & 0 deletions test/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,11 @@ createKeySpec() {
expectEquals(new Key(Car, Turbo), new Key(Car, Turbo), true);
});

it('should not be equal to another key where type and annotation are same '
'but reversed', () {
expectEquals(new Key(Car, Turbo), new Key(Turbo, Car), false);
});

it('should not be equal to another key if types are different', () {
expectEquals(new Key(Car), new Key(Porsche), false);
});
Expand Down

2 comments on commit 0930b37

@vicb
Copy link
Contributor

@vicb vicb commented on 0930b37 May 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, huge penalty!
Have you reported this to the Dart dev team ?

@mvuksano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you even figure this one out? @pavel

Please sign in to comment.