Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

perf($q): move Deferred and Promise methods to prototypes#8437

Closed
jeffbcross wants to merge 3 commits intoangular:masterfrom
jeffbcross:prototype-q
Closed

perf($q): move Deferred and Promise methods to prototypes#8437
jeffbcross wants to merge 3 commits intoangular:masterfrom
jeffbcross:prototype-q

Conversation

@jeffbcross
Copy link
Copy Markdown
Contributor

Opening PR for tests and comparison with other solutions for $q perf.

NOTE: Deferred doesn't get all the advantages of moving methods to the prototype,
since the constructor binds instance methods to "this" to support unbounded execution.

NOTE: This change increases the gzipped version by 100 bytes.

NOTE: Deferred doesn't get all the advantages of moving methods to the prototype,
since the constructor binds instance methods to "this" to support unbounded execution.

NOTE: This change increases the gzipped version by 100 bytes.
@jeffbcross
Copy link
Copy Markdown
Contributor Author

Most recent JSPerf tests on my Macbook Air yielded 88949 ops/sec with existing implementation, and 158503 ops/sec with this refactor (78% increase). Note: I recently changed the benchmarks to call jsperf's deferred.resolve directly, without using .call.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I prefer the settlePromiseFromHandler method from Bluebird --- we save a few bytes by not re-creating these wrapped callbacks for every then().

This might be good enough for now, but you can see we're creating a lot of duplicate code here and not reusing much.

@jeffbcross
Copy link
Copy Markdown
Contributor Author

@caitp I added some commits and addressed your comments. Let me know if you think this is good to merge.

@caitp
Copy link
Copy Markdown
Contributor

caitp commented Aug 1, 2014

I'd still prefer to address https://github.com/angular/angular.js/pull/8437/files#r15715785 --- but worry about that later, it looks good to me.

@jeffbcross
Copy link
Copy Markdown
Contributor Author

Landed as 23bc92b

@jeffbcross jeffbcross closed this Aug 4, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants