search

thriftrw / thriftrw-node

A thrift binary encoding library using bufrw
MIT License
57 stars 25 forks source link

revert thrift-idl cache option #145

Closed Matt-Esch closed 7 years ago

Matt-Esch commented 7 years ago

Updating thriftrw beyond 3.1.0 caused a serious regression in cpu and memory usage during thrift client creation. I bisected it down to the enabling of the cache option. Reverting the cache option significantly improves performance. The impact of the cache option is to add 10 minutes to our test suite and increase startup time by 10s.

Unless this was added to address a different observed concern, I'm removing the cache option.

CLAassistant commented 7 years ago

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Matt Esch seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

Matt-Esch commented 7 years ago

cc @kriskowal @Raynos

Raynos commented 7 years ago

lgtm.

kriskowal commented 7 years ago

Whelp, I added the cache option specifically so you could use it to improve perf, so I’m fine backing it out if it hurts more than it can help.