search

bmuller / txyam

Yet Another Memcached (YAM) client for Python Twisted
Other
10 stars 5 forks source link

Update client.py #6

Closed aliowka closed 10 years ago

aliowka commented 10 years ago

getClient is called many times, so hashes are recalculated again and again.. Please consider to put HashRing function in init so it's called only once.. This change improves perfomance by x10 with only two hosts given.. Thanks a lot for the open source!!!

bmuller commented 10 years ago

Thanks for the patch. The issue is that one or more clients may have disconnected, so you need to check if the ring should be rebuilt in getClient if you're going to cache it. If you add that check then I'd be happy to merge the PR.

Thanks!

B

On Oct 6, 2014, at 04:25, aliowka notifications@github.com wrote:

getClient is called many times, so hashes are recalculated again and again.. Please consider to put HashRing function in init so it's called only once.. This change improves perfomance by x10 with only two hosts given.. Thanks a lot for the open source!!!

You can merge this Pull Request by running

git pull https://github.com/aliowka/txyam patch-1 Or view, comment on, or merge it at:

https://github.com/bmuller/txyam/pull/6

Commit Summary

Update client.py File Changes

M txyam/client.py (3) Patch Links:

https://github.com/bmuller/txyam/pull/6.patch https://github.com/bmuller/txyam/pull/6.diff — Reply to this email directly or view it on GitHub.

aliowka commented 10 years ago

Hi Brian , you are right.. I'm working on it.. Thanks for the patch. The issue is that one or more clients may have disconnected, so you need to check if the ring should be rebuilt in getClient if you're going to cache it. If you add that check then I'd be happy to merge the PR.

Thanks!

B

On Oct 6, 2014, at 04:25, aliowka notifications@github.com wrote:

getClient is called many times, so hashes are recalculated again and again.. Please consider to put HashRing function in init so it's called only once.. This change improves perfomance by x10 with only two hosts given.. Thanks a lot for the open source!!!

You can merge this Pull Request by running

git pull https://github.com/aliowka/txyam patch-1 Or view, comment on, or merge it at:

https://github.com/bmuller/txyam/pull/6

Commit Summary

Update client.py File Changes

M txyam/client.py (3) Patch Links:

https://github.com/bmuller/txyam/pull/6.patch https://github.com/bmuller/txyam/pull/6.diff — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/bmuller/txyam/pull/6#issuecomment-58016564.

bmuller commented 10 years ago

So this was a much bigger performance hit than I realized - and I fixed in the latest version. Thanks so much for finding this!