1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.04k stars 241 forks source link

Prefer currying instead of .bind(this) #356

Closed pocesar closed 8 years ago

pocesar commented 10 years ago

v8 have an infamous underoptimization of Function.bind implementation, that can lead to serious performance issues, as much as 20 times slower (you can see the Chrome tests in here http://jsperf.com/bind-vs-closure-setup/17) in which the binder closure is much faster than test.bind(self). It's a little more verbose, but the performance gains are clear, and it's not by a small margin...

bind     test_bind();       4,442,287   ±1.88%
binder  test_binder();   92,674,939   ±2.20%
anatoliychakkaev commented 10 years ago

+1

On Sun, Jan 12, 2014 at 5:28 PM, Paulo Cesar notifications@github.comwrote:

v8 have an infamous underoptimization of Function.bind implementation, that can lead to serious performance issues, as much as 20 times slower (you can see the Chrome tests in here http://jsperf.com/bind-vs-closure-setup/17) in which the binder closure is much faster than test.bind(self). It's a little more verbose, but the performance gains are clear, and it's not by a small margin...

bind test_bind(); 4,442,287 ±1.88% binder test_binder(); 92,674,939 ±2.20%

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/356 .

randunel commented 10 years ago

I tend to regard the .bind issue as a micro-optimization. You could have a greater performance impact by optimizing network resource utilization, such as not using whole objects (partial reads, partial writes) when dealing with data reads and updates, and completely dropping useless reads / writes. But I agree that a benchmark-test would be helpful in this debate.

pocesar commented 10 years ago

this change would also make it easier for a promise based approach to be achieve (as I talked with @anatoliychakkaev on #375)

pocesar commented 10 years ago

I'm doing a heavy cleanup on my promise version of Jugglingdb, and already replaced all the function.bind with the curry versions

https://github.com/pocesar/promised-jugglingdb/commit/1b7bddb5c418c18ddcb897a02c59e62fc08c9feb#source

then I'll push the promise based version. all tests are passing (i've rewrote most of the tests to use expect though)