Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly batch Client.set_many() calls #182

Merged
merged 2 commits into from
Aug 30, 2018

Conversation

shargan
Copy link
Contributor

@shargan shargan commented Aug 30, 2018

pymemcache.client.base.Client.set_many() currently loops over its values dictionary and sends each key-value pair separately rather. This diff drops them in bulk into the fewest TCP packets in which they'll fit. If the overall approach meets your approval, I'll send a separate PR to do the same for delete_many().

Using the expanded benchmarks from #180 and snipping output down to just the relevant lines, the current codebase yields these results:

# python2.7
$ pytest --verbose --capture=no --no-cov -m benchmark pymemcache/test --keys 100 --count 10000
========================================== test session starts ===========================================
platform darwin -- Python 2.7.15, pytest-3.7.4, py-1.6.0, pluggy-0.7.1 -- /Users/shargan/repos/pymemcache/.venv/bin/python

pymemcache/test/test_benchmark.py::test_bench_set[pymemcache] 0.449674129486
pymemcache/test/test_benchmark.py::test_bench_set_multi[pymemcache] 36.8027918339


# python3.6
$ pytest --verbose --capture=no --no-cov -m benchmark pymemcache/test --keys 100 --count 10000
========================================== test session starts ===========================================
platform darwin -- Python 3.6.6, pytest-3.7.4, py-1.6.0, pluggy-0.7.1 -- /Users/shargan/repos/pymemcache/.venv36/bin/python3.6

pymemcache/test/test_benchmark.py::test_bench_set[pymemcache] 0.3536701202392578
pymemcache/test/test_benchmark.py::test_bench_set_multi[pymemcache] 35.68993163108826

With this diff applied, the results show increased set_multi throughput:

# python2.7
$ pytest --verbose --capture=no --no-cov -m benchmark pymemcache/test --keys 100 --count 10000
========================================== test session starts ===========================================
platform darwin -- Python 2.7.15, pytest-3.7.4, py-1.6.0, pluggy-0.7.1 -- /Users/shargan/repos/pymemcache/.venv/bin/python

pymemcache/test/test_benchmark.py::test_bench_set[pymemcache] 0.389505147934
pymemcache/test/test_benchmark.py::test_bench_set_multi[pymemcache] 10.9233689308


# python3.6
$ pytest --verbose --capture=no --no-cov -m benchmark pymemcache/test --keys 100 --count 10000
========================================== test session starts ===========================================
platform darwin -- Python 3.6.6, pytest-3.7.4, py-1.6.0, pluggy-0.7.1 -- /Users/shargan/repos/pymemcache/.venv36/bin/python3.6

pymemcache/test/test_benchmark.py::test_bench_set[pymemcache] 0.38876795768737793
pymemcache/test/test_benchmark.py::test_bench_set_multi[pymemcache] 11.447095155715942

This has the added side effect of allowing future revisions to add X_many() sister methods to any of the other storage commands.

@cgordon
Copy link
Collaborator

cgordon commented Aug 30, 2018

The code changes look good, and I'm excited to have that ancient TODO done. It looks like Travis is failing due to a few simple formatting issues, so once those are fixed I'll merge. Thanks!

@shargan
Copy link
Contributor Author

shargan commented Aug 30, 2018

Ah, missed those somehow. Thanks, flake8 complaints are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants