Optimize IO thread offload for modified argv

[uriyage]

Improve expired commands performance with IO threads

Background

In our IO threads architecture, IO threads allocate client argv's and later when we free it after processCommand we offload its free to the IO threads.
With jemalloc, it's crucial that the same thread that allocates memory also frees it.

For some commands we modify the client's argv in the main thread during command processing (for example in SET EX command we rewrite the command to use absolute time for replication propagation).

Current issues

1. When commands are rewritten (e.g., expire commands), we store the original argv
   in c->original_argv. However, we're currently:
   • Freeing new argv (allocated by main thread) in IO threads
   • Freeing original argv (allocated by IO threads) in main thread
2. Currently, c->original_argv points to new array with old
   objects, while c->argv has old array with new objects, making memory free management complicated.

Changes

1. Refactored argv modification handling code to ensure consistency - both array and objects are now either all new or all old
2. Moved original_argv cleanup to happen in resetClient after argv cleanup
3. Modified IO threads code to properly handle original argv cleanup when argv are modified.

Performance Impact

Benchmark with SET EX commands (650 clients, 512 byte value, 8 IO threads):

• New implementation: 729,548 ops/sec
• Old implementation: 633,243 ops/sec
  Representing a ~15% performance improvement due to more efficient memory handling.

[zuiderkwast]

Very nice! Just some points I'm not sure if we can improve, but I'm not sure we have to change anything. Maybe not. Let's think about and discuss first.

[zuiderkwast]

Just noting that this function became pretty complex now. It would be useful to have unit test for it, but I will not require it. I hope we have some coverage for the edge cases, like a test that does replication of RESTORE with TTL and without ABSTTL. (It does redactClientCommandArgument twice and adds ABSTTL to the end.)

[zuiderkwast]

Here, it is clear what the arguments mean, but at the call site, it is not immediately clear to the reader what the 2nd argument is, when you see the call like tryOffloadFreeArgvToIOThreads(c, 0).

Trying to think of ideas:

• We could use some enum {FREE_ARGV, FREE_ORIGINAL_ARGV} instead of 0 and 1.
• We could pass pointers to the argv and argc fields within the client struct instead of the current parameters, so tryOffloadFreeArgvToIOThreads(robj ***argv_ptr, int *argc_ptr) and then at the call site it would look like tryOffloadFreeArgvToIOThreads(&c->original_argv, &c->original_argc). Is this good?