follow up on ways to reduce allocation on multi_get #45
Conversation
|
Note: this is the optimized get multi only use for single server with raw mode... we should run tests in deployed systems (staging) before merging |
| @@ -249,7 +250,7 @@ def next_line_to_tokens | |||
|
|
|||
| def read_data(data_size) | |||
| resp_data = @io_source.read(data_size) | |||
| @io_source.read(TERMINATOR.bytesize) | |||
| @io_source.read_to_outstring(TERMINATOR.bytesize, @terminator_buffer) | |||
There was a problem hiding this comment.
this removes an additional allocation for all the non multi calls
…what is supported for simplified fast get multi
|
OK, I finally had some time to capture data on this. Setting up two staging environments and running 1:1 traffic between them. The differences are small but seem to be better across all metrics with this change. Reduced object allocations, CPU, and request time. With no increases in errors. Data available in internal drive "Dalli Object Allocations" |
| @@ -150,10 +150,10 @@ def quiet? | |||
| alias multi? quiet? | |||
|
|
|||
| # NOTE: Additional public methods should be overridden in Dalli::Threadsafe | |||
| ALLOWED_QUIET_OPS = %i[add replace set delete incr decr append prepend flush noop].freeze | |||
There was a problem hiding this comment.
I'm not understanding the reason for making this public (at least as part of this PR)?
There was a problem hiding this comment.
rubocop update complained about private constant.
| TERMINATOR = "\r\n" | ||
| META_NOOP = "mn\r\n" | ||
| META_NOOP_RESP = 'MN' |
There was a problem hiding this comment.
ah these got refactored away, I can remove them
| TERMINATOR = "\r\n" | ||
| META_NOOP = "mn\r\n" | ||
| META_NOOP_RESP = 'MN' | ||
| META_VALUE_RESP = 'VA' |
Ok, this is a bit silly, but after our conversation I was curious how much we could reduce allocations in the get multi tight loop... This does reduce allocations which is nice, but the overall benchmark doesn't show much of a difference... Still kind of fun to poke around at, and it might be nice to add a check to CI that ensures we keep pushing allocations down for our commands.
allocations before:
1944allocations after:
1659benchmark before:
benchmark after:
paired with Aaron and was able to cut allocations in half... while it still doesn't show up as much in a micro benchmark this should add up across all cache calls across all web requests. The new method is a bit harder to understand, but feels worth it for such a frequently used tight loop.