rueidis icon indicating copy to clipboard operation
rueidis copied to clipboard

feat : Implement slot-based MGET batching for cluster clients

Open SoulPancake opened this issue 2 months ago • 8 comments

For #844

SoulPancake avatar Oct 04 '25 08:10 SoulPancake

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

jit-ci[bot] avatar Oct 04 '25 08:10 jit-ci[bot]

I optimised the slice mem allocation, is there any other potential improvement I can work on? @rueian

SoulPancake avatar Oct 10 '25 05:10 SoulPancake

I optimised the slice mem allocation, is there any other potential improvement I can work on? @rueian

I wonder if it is possible to avoid using MGets since it allocates a make(map[uint16]Completed, 8) though I am not sure if it is allocated on the heap or the stack after compiler optimization. Would you mind checking that? If it is allocated on the stack, then it should be fine.

rueian avatar Oct 10 '25 18:10 rueian

I wonder if it is possible to avoid using MGets since it allocates a make(map[uint16]Completed, 8) though I am not sure if it is allocated on the heap or the stack after compiler optimization. Would you mind checking that? If it is allocated on the stack, then it should be fine.

@rueian make(map[uint16]Completed, 8) in MGets/JsonMGets does escape to heap. What do you suggest we do here

SoulPancake avatar Oct 20 '25 07:10 SoulPancake

What do you suggest we do here

  1. Try inline MGets/JsonMGets manually.
  2. Then replacing make(map[uint16]Completed, 8) with make(map[uint16]int, 8) where the values are the indexes of cmds.s, eliminating the use of slotCmds.

rueian avatar Oct 20 '25 16:10 rueian

I have some trouble will my personal laptop, I will address these in a week approx, hope that is okay

SoulPancake avatar Oct 21 '25 17:10 SoulPancake

@rueian I inlined the grouping logic:

  1. Added CRC16 hashing and slot calculation functions (copied from internal/cmds/slot.go) to helper.go for key-to-slot mapping. Let me know if I should move this to a common util?
  2. Replaced the intl.MGets/intl.JsonMGets calls with manual key grouping into a map[uint16][]string.
  3. Built the command slice (cmds.s) directly using the client builder (client.B().Mget().Key(group...).Build().Pin() for MGet, and similarly for JsonMGet with path).
  4. Maintained a parallel cmdKeys slice to track keys per command for response parsing.
  5. Eliminated the intermediate slotCmds map of Completed structs, reducing heap pressure while preserving functionality.

SoulPancake avatar Oct 29 '25 10:10 SoulPancake

Hi @rueian Any chance to review this?

SoulPancake avatar Nov 05 '25 13:11 SoulPancake