jeremyckahn / shifty

The fastest TypeScript animation engine on the web
https://jeremyckahn.github.io/shifty/doc/
MIT License
1.54k stars 88 forks source link

Length checks in some areas of the library could bring some speedups/improvements #73

Closed ghost closed 6 years ago

ghost commented 9 years ago

Putting length checks in some methods could bring some speed improvements. For instance, in the "applyFilter" method, before calling "each" we should first check that the length of "filters" is not 0. It should be both a speed-wise and a memory-wise improvement (the interpreter does not allocate the required resources to bring up the "each" iteration if not required). There are some more places in the code that could benefit from this check. It's less resourceful to compare length property to 0 than to fire up a whole function and then not execute it. Just my point of view. Let me know what you think about this.

jeremyckahn commented 9 years ago

Hi @adrianvoica, thanks for your suggestion. This is something I've thought about in the past, but opted not to pursue because the tradeoff between potential performance gains and additional code didn't seem worth it. Additionally, in cases like the applyFilter scenario you mentioned, the code would actually be slowed down (though not significantly) by the attempt to optimize the work pipeline most of the time (in the standard Shifty build, there is always a filter).

Given that this is a performance question, it can only be answered by measurements and data. I'd be okay with adding the checks that you suggest if we can quantify that they do in fact improve performance for common scenarios. Perhaps this is something that jsperf would be useful for?

ghost commented 9 years ago

Yeah, sure. It's up to you to decide if you want to sacrifice a bit of code legibility to performance, but we could bring up some real measurements from jsperf. But what I am talking about is real visual performance (that's actually quantifiable in the Chrome/WebKit/etc. Dev Tools).

jeremyckahn commented 9 years ago

I'm totally open to making these changes for the sake of performance — I just want to make sure that we're getting the gains that we're hoping for. Let's leave this open for now.

jeremyckahn commented 6 years ago

Since this issue hasn't seen any activity in close to three years and 2.0 has introduced some performance improvements, I am going to close this. I'm happy to reopen if anyone is interested in putting together a PR, though!