Implement build cmd in rust #56
Conversation
bisho
force-pushed
the
rust_parse_header
branch
from
c4b711c to
309bcc3
Compare
| @@ -1186,7 +1183,7 @@ def test_delta_cmd(memcache_socket: MemcacheSocket, cache_client: CacheClient) - | |||
|
|
|||
| cache_client.delta(key=Key("foo"), delta=1, refresh_ttl=60, no_reply=True) | |||
| memcache_socket.sendall.assert_called_once_with( | |||
| b"ma foo q D1 T60\r\n", with_noop=True | |||
| b"ma foo q T60 D1\r\n", with_noop=True | |||
There was a problem hiding this comment.
Looks like order on the wire doesn't matter for the flags, nothing here specifies a required order: https://github.com/memcached/memcached/blob/master/doc/protocol.txt#L116 so this seems ok 👍
There was a problem hiding this comment.
Python 3.11 was in fact generating random order and we were running tests in python 3.8 with fixed seed to prevent that. The rust implementation guarantees ordering, not very important, but simplifies testing.
| @@ -560,7 +557,7 @@ def test_get_cmd(memcache_socket: MemcacheSocket, cache_client: CacheClient) -> | |||
| ) | |||
| assert_called_once_with_command( | |||
| memcache_socket.sendall, | |||
| b"mg lCV3WxKxtWrdY4s1+R710+9J b t l v h f R30 T300\r\n", | |||
| b"mg w7puw63Dp29k4o23 b t l v h f R30 T300\r\n", | |||
There was a problem hiding this comment.
Any undocumented implications of no longer base64 encoding unicode keys?
Looks like this will mean a new warmup & double storing cache fights during deploys, but should be ok if all documented.
There was a problem hiding this comment.
This is still base64. The change is that now we won't generate a blake2b hash blindly for any unicode, only for large keys.
I also didn't like how the Key() class ended looking before. It is a burden to: 1) not allow binary keys, forcing users to encode themselved. 2) mark unicode presence on keys (that was mostly never used).
Now we will allow bytes & any unicode and be able to encode it properly. Before it was expensive to check if something wasn't just ascii, required iterating each char thus the is_unicode flag, but with this rust implementation you just pass bytes to it, and if any char is not ascii it will b64 it and add the binary flag with minimal overhead. I very much like best this abstraction.
Regarding the compatibility, this will be a major since we also break the meta-commands api completely, with the new way of passing flags. I'll fix docs and document this behavior change. The high-level api is unaffected, so I hope most users don't see many problems to upgrade.
bisho
force-pushed
the
rust_parse_header
branch
from
309bcc3 to
8358df9
Compare
bisho
force-pushed
the
rust_build_cmd
branch
3 times, most recently
from
3d6a61f to
7813cc8
Compare
## Motivation / Description The second cpu-intensive part of the request processing is building the cmd. Also instead of building dicts of flags we can use a single flags object, which also simplifies the API of the lower commands. I chose to still built a single flags object, but we could explore building one flags object per meta-command, as the flags that they support differ, and it could lead to a more type-safe low-level implementation. ## Performance: * Initial: multithreaded: Overall: 110779.55 RPS / 9.03 us/req singlethreaded: Overall: 111545.63 RPS / 8.96 us/req * Rust only for response parsing multithreaded: Overall: 245898.34 RPS / 4.07 us/req singlethreaded: Overall: 246165.19 RPS / 4.06 us/req * Now (rust also for build_cmd) multithreaded: Overall: 319587.03 RPS / 3.13 us/req singlethreaded: Overall: 323101.77 RPS / 3.10 us/req
Build cmd in rust
Motivation / Description
The second cpu-intensive part of the request processing is building
the cmd.
Also instead of building dicts of flags we can use a single
flags object, which also simplifies the API of the lower
commands.
I chose to still built a single flags object, but we could
explore building one flags object per meta-command, as the
flags that they support differ, and it could lead to a more
type-safe low-level implementation.
Performance:
Initial (before all this patchsets):
multithreaded: Overall: 110779.55 RPS / 9.03 us/req
singlethreaded: Overall: 111545.63 RPS / 8.96 us/req
Rust only for response parsing
multithreaded: Overall: 245898.34 RPS / 4.07 us/req
singlethreaded: Overall: 246165.19 RPS / 4.06 us/req
Now (rust also for build_cmd)
multithreaded: Overall: 319587.03 RPS / 3.13 us/req
singlethreaded: Overall: 323101.77 RPS / 3.10 us/req
Speedscope:
Before: 1.18us
After: 456ns
