perf($injector) #5388
perf($injector) #5388
Conversation
|
so it shrinks code and you see a slight performance increase on certain browsers, that's good stuff =) How many browsers have you run the suite on so far? I would be most concerned about mobile browsers, and I'm fairly limited to which mobile browsers I can test with, so it's hard to really make a concrete statement about it. |
|
By my tests you see slightly better performance with the switch method on android webview on a Nexus S, while apply is significantly faster on Android Chrome on the same device. I haven't reviewed the tests themselves yet, though. |
ghost
assigned 
|
I just tested on nexus 5 / android 4.4 / chrome. Results are similar, about a 50% speedup. |
|
I tested it in the real world and I see only about 5% speedup for execution time of invoke calls. overall speedup is totally insignificant. even though the speedup is not significant, I'm happy to delete a whole bunch of code that do what it's supposed to do - speed up the injection. |
|
just fyi: this change saves 120 bytes from min+gzip build |
|
Slimming down the angularjs build, one tweet at a time |
|
Wait! This was my favourite code from Angular codebase. Anytime I saw Misko having this triangle on his screen, I would know it was injector... I'm sad. |
|
At least you still have https://github.com/angular/angular.js/blob/master/test/auto/injectorSpec.js?source=cc#L39 |
I did some testing on switch vs apply: http://jsperf.com/angularjs-invoke-apply-vs-switch
The existing benchmark always made calls with the same number of args (4), so I assume branch prediction is what made that so quick. (http://jsperf.com/apply-vs-call-vs-invoke/30)
However, in my benchmark, each invoke call has a different (random) number of arguments, which I feel is closer to real world conditions. Switching back to just
applyseems to be about 40% faster, and saves a good bit of code as well.It's also possible I've messed up the benchmark.
Thoughts?
(edited typo in speedup percentage)