search

dmfay / massive-js

A data mapper for Node.js and PostgreSQL.
2.49k stars 158 forks source link

Database.prototype.clone performance bottleneck #673

Closed pauldprice closed 5 years ago

pauldprice commented 5 years ago

Following the pg-promise best practice of chaining queries, I began using massive's withConnection function. I immediately noticed a pronounced performance degradation. Chaining queries should yield a more efficient use of database connections, not a slowdown; I am clocking a 10x slowdown in many of my tests.

The bottleneck is in Database.prototype.clone, which uses lodash to do a deep clone while capturing a copy of the newly cloned 'executable' objects for later reassignment of their db property to the freshly cloned database object. Admittedly, I'm not familiar with the guts of massive-js and don't know enough to feel comfortable with the following "fix". However, I believe the nested call to _.cloneDeepWith can be replaced with _.cloneWith. This change may naively introduce other issues, but the massive-js test suite ran flawlessly. The performance improvement is significant.

I'm happy to submit a PR with this change, but thought that you might be able to quickly tell me why the change will not work. In any case, if you are too busy to dig into this problem, please point me in the right direction and I'll run with it. Thank you!

dmfay commented 5 years ago

I did a little digging and I think you're right that the inner clone doesn't need to be recursive! Fire away with the pull request whenever you're ready :)

pauldprice commented 5 years ago

Thanks for confirming - and thanks for maintaining this project! I submitted PR #674 to address this issue.