refactor(server): pass ParsedArgs through command dispatch

[romange]

Summary

Thread facade::ParsedArgs through command invocation paths so dispatch no longer bounces through CmdArgList scratch buffers.

Changes

• Update Service::InvokeCmd, CommandId::Invoke, slowlog recording, EXEC, EVAL async storage, and squashed dispatch to use ParsedArgs directly.
• Convert DEBUG subcommand helpers to consume the existing CmdArgParser instead of rebuilding argument slices.
• Remove obsolete ParsedArgs::ToSlice/ToVec and related scratch helpers.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Refactor command dispatch to pass ParsedArgs end-to-end

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Thread facade::ParsedArgs through dispatch/invoke paths to avoid ArgSlice scratch buffers.
• Update slowlog, EXEC/EVAL storage, and squashed dispatch to operate on ParsedArgs.
• Refactor DEBUG subcommands to consume CmdArgParser directly; remove ParsedArgs slice helpers.

Diagram

graph TD
  PA[("ParsedArgs")] --> SD["Service::DispatchCommand"] --> SI["Service::InvokeCmd"] --> CI["CommandId::Invoke"] --> CP["CmdArgParser"]
  SD --> SC["StoredCmd (EXEC/EVAL)"] --> SI
  SD --> MS["MultiCommandSquasher"] --> SI
  SI --> SL["SlowLog.Add"]

Loading

High-Level Assessment

The chosen approach (threading ParsedArgs through the entire dispatch/invoke surface) is the most robust way to eliminate repeated ArgSlice/CmdArgVec scratch conversions while preserving correct lifetime semantics via BackedArguments where needed (StoredCmd, deferred replies). Alternatives like continuing to pass ArgSlice and rebuilding scratch spans would keep hidden allocations/copies and make lifetime handling more error-prone.

Files changed (15) +127 / -183

Refactor (14) +124 / -177

cmd_arg_parser.hRemove ArgSlice ctor; accept ParsedArgs directly +0/-3

Remove ArgSlice ctor; accept ParsedArgs directly

• Drops the CmdArgParser constructor that took ArgSlice and standardizes construction from BackedArguments or ParsedArgs. This aligns the parser API with the new end-to-end ParsedArgs dispatch flow.

src/facade/cmd_arg_parser.h

facade_types.hRemove ParsedArgs ToSlice/ToVec scratch conversion helpers +0/-28

Remove ParsedArgs ToSlice/ToVec scratch conversion helpers

• Deletes ParsedArgs::ToSlice/ToVec and per-variant implementations that materialized CmdArgVec scratch buffers. ParsedArgs is now used as the primary argument carrier with iteration/indexing instead of conversion APIs.

src/facade/facade_types.h

command_registry.hInvoke handlers with ParsedArgs; simplify handler wiring +5/-7

Invoke handlers with ParsedArgs; simplify handler wiring

• Updates CommandId::Invoke to take ParsedArgs and construct CmdArgParser from it. Simplifies SetHandler overloads (removes async_support boolean in the generic handler setter).

src/server/command_registry.h

conn_context.ccStore/instrument commands using ParsedArgs (StoredCmd + slowlog latency) +14/-9

Store/instrument commands using ParsedArgs (StoredCmd + slowlog latency)

• Adds a StoredCmd constructor that deep-copies from ParsedArgs into BackedArguments. Updates CommandContext::RecordLatency to accept ParsedArgs and rewrites slowlog argument rewriting (EXEC/EVAL) without mutating an ArgSlice prefix in-place.

src/server/conn_context.cc

conn_context.hUpdate stored command and latency APIs to ParsedArgs +3/-6

Update stored command and latency APIs to ParsedArgs

• Removes CmdRef/StoredCmd ArgSlice slicing helpers and exposes ParsedArgs directly. Updates CommandContext::RecordLatency signature to accept ParsedArgs.

src/server/conn_context.h

debugcmd.ccRefactor DEBUG subcommands to consume CmdArgParser directly +63/-68

Refactor DEBUG subcommands to consume CmdArgParser directly

• Stops rebuilding CmdArgList scratch slices from UnparsedArgs; instead advances the existing CmdArgParser via Next/Peek/UnparsedArgs. Updates all DEBUG subcommand helpers accordingly and fixes parsing/error handling to use parser.TakeError().

src/server/debugcmd.cc

debugcmd.hChange DEBUG helper signatures from CmdArgList to CmdArgParser +11/-11

Change DEBUG helper signatures from CmdArgList to CmdArgParser

• Updates private DEBUG subcommand helper method signatures to take CmdArgParser, matching the new parsing style in debugcmd.cc.

src/server/debugcmd.h

generic_family.ccReplace ParsedArgs ToVec usage with direct iteration +2/-1

Replace ParsedArgs ToVec usage with direct iteration

• Updates FieldExpire to build the fields vector by iterating over ParsedArgs (Tail + assign) instead of calling the removed ParsedArgs::ToVec helper.

src/server/generic_family.cc

main_service.ccDispatch/Invoke/EXEC/EVAL paths use ParsedArgs directly +12/-23

Dispatch/Invoke/EXEC/EVAL paths use ParsedArgs directly

• Updates DispatchCommand to set CommandContext tail args as ParsedArgs where possible, only materializing backing storage for deferred replies. Updates EXEC key-deduction, exec-body invocation, and async EVAL command storage to avoid ArgSlice scratch conversions.

src/server/main_service.cc

main_service.hInvokeCmd now takes ParsedArgs +1/-1

InvokeCmd now takes ParsedArgs

• Changes Service::InvokeCmd signature to accept ParsedArgs, reflecting the refactored dispatch and call chain.

src/server/main_service.h

multi_command_squasher.ccSquasher operates on CmdRef.args (ParsedArgs) with no temp keylist +8/-13

Squasher operates on CmdRef.args (ParsedArgs) with no temp keylist

• Removes per-command slicing into a temporary CmdArgVec and uses ParsedArgs directly for validation, key extraction, transaction init, and invocation in both standalone and squashed-hop paths.

src/server/multi_command_squasher.cc

multi_command_squasher.hRemove tmp_keylist_ scratch storage +0/-1

Remove tmp_keylist_ scratch storage

• Deletes the tmp_keylist_ member that existed solely to back ArgSlice conversions for squashing. The squasher now relies on ParsedArgs throughout.

src/server/multi_command_squasher.h

slowlog.ccSlowlog Add accepts ParsedArgs and indexes args directly +2/-2

Slowlog Add accepts ParsedArgs and indexes args directly

• Updates SlowLogShard::Add to take ParsedArgs and fetch arguments via operator[] instead of facade::ArgS on CmdArgList.

src/server/slowlog.cc

slowlog.hUpdate SlowLogShard::Add signature to ParsedArgs +3/-4

Update SlowLogShard::Add signature to ParsedArgs

• Removes the CmdArgList alias usage and updates the slowlog API to accept ParsedArgs directly.

src/server/slowlog.h

Tests (1) +3 / -6

cmd_arg_parser_test.ccSwitch parser tests to BackedArguments storage +3/-6

Switch parser tests to BackedArguments storage

• Reworks test fixture argument storage to use cmn::BackedArguments instead of a std::vector + CmdArgVec scratch span. This matches the updated CmdArgParser construction API.

src/facade/cmd_arg_parser_test.cc

[augmentcode]

🤖 Augment PR Summary

Summary: Refactors server command dispatch to pass facade::ParsedArgs end-to-end, avoiding repeated conversions through scratch CmdArgList/CmdArgVec buffers.

Changes:

• Update core dispatch points (Service::InvokeCmd, CommandId::Invoke) to accept ParsedArgs directly.
• Thread ParsedArgs through slowlog recording, EXEC processing, EVAL async command storage, and multi-command squashing.
• Convert DEBUG subcommand handling to consume the existing CmdArgParser state instead of rebuilding argument slices.
• Remove obsolete ParsedArgs::ToSlice/ToVec helpers and adjust call sites to iterate/assign directly.
• Adjust CmdArgParser unit tests to use cmn::BackedArguments as the backing store.

Technical Notes: The refactor relies on ParsedCommand owning BackedArguments so ParsedArgs can be safely stored for deferred/async execution paths.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. ParsedArgs assign reallocations 🐞 Bug ➹ Performance

Description

The PR introduced std::vector::assign(begin,end) over ParsedArgs iterators, but
ParsedArgs::const_iterator is declared as an input iterator, so vector::assign cannot pre-size and
may grow incrementally. This can regress allocation/CPU in hot paths like DispatchCommand
deferred-reply handling (and similarly in FieldExpire).

Code

src/server/main_service.cc[R1487-1488]

+    cmd_cntx->arg_slice_backing.assign(args_no_cmd.begin(), args_no_cmd.end());  // Ensure lifetime
+    cmd_cntx->SetTailArgs(facade::ArgSlice{cmd_cntx->arg_slice_backing});

Evidence

The impacted copies iterate using ParsedArgs::const_iterator, which is explicitly marked as an input
iterator; the PR introduced vector::assign(begin,end) over that iterator in multiple command paths.
That combination prevents vector from reliably pre-sizing, unlike the previous ToVec path that
reserved based on size.

src/facade/facade_types.h[81-120]
src/server/main_service.cc[1486-1491]
src/server/generic_family.cc[2291-2300]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`std::vector::assign(first,last)` is now used with `ParsedArgs::begin()/end()`, but `ParsedArgs::const_iterator` is declared as `std::input_iterator_tag`. With input iterators, `vector::assign` cannot pre-compute distance to size/reserve once, so it may append incrementally and cause extra reallocations on larger arg lists.

### Issue Context
This regression was introduced when removing `ParsedArgs::ToVec` and replacing it with `vector::assign` over ParsedArgs iterators.

### Fix options
- **Option A (localized, minimal risk):** Replace `assign(begin,end)` with `clear(); reserve(parsed.size()); for (auto sv : parsed) push_back(sv);` in the few hot sites.
- **Option B (systemic):** Change `ParsedArgs::const_iterator::iterator_category` to `std::forward_iterator_tag` (the iterator is index-based and multi-pass), allowing standard library code paths to pre-size when possible.

### Fix Focus Areas
- src/server/main_service.cc[1486-1491]
- src/server/generic_family.cc[2291-2300]
- src/facade/facade_types.h[81-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[dranikpg]

I'll use this in #7648