refactor(server): pass ParsedArgs through command dispatch

src/facade/cmd_arg_parser.h

@@ -206,9 +206,6 @@ struct CmdArgParser {
   };
 
  public:
-  explicit CmdArgParser(ArgSlice args) : args_{args} {
-  }
-
   explicit CmdArgParser(const cmn::BackedArguments& bargs, uint32_t offset = 0)
       : args_{bargs, offset} {
   }

src/facade/cmd_arg_parser_test.cc

@@ -20,16 +20,13 @@ namespace facade {
 class CmdArgParserTest : public testing::Test {
  public:
   CmdArgParser Make(absl::Span<const std::string_view> args) {
-    storage_.assign(args.begin(), args.end());
-    arg_vec_.clear();
-    for (auto& s : storage_)
-      arg_vec_.push_back(MutableSlice{s.data(), s.size()});
-    return CmdArgParser{absl::MakeSpan(arg_vec_)};
+    storage_.Assign(args.begin(), args.end(), args.size());
+    return CmdArgParser{storage_};
   }
 
  private:
   CmdArgVec arg_vec_;
-  std::vector<std::string> storage_;
+  cmn::BackedArguments storage_;
 };
 
 TEST_F(CmdArgParserTest, BasicTypes) {

src/facade/facade_types.h

@@ -127,14 +127,6 @@ class ParsedArgs {
     return const_iterator{this, size()};
   }
 
-  ArgSlice ToSlice(CmdArgVec* scratch) const {
-    return std::visit([scratch](const auto& args) { return args.ToSlice(scratch); }, args_);
-  }
-
-  void ToVec(CmdArgVec* vec) const {
-    std::visit([vec](const auto& args) { return args.ToVec(vec); }, args_);
-  }
-
  private:
   struct WrapperBacked {
     WrapperBacked(const cmn::BackedArguments* args, uint32_t index = 0)  // NOLINT
@@ -163,18 +155,6 @@ class ParsedArgs {
     std::string_view at(size_t i) const {
       return args_->at(index_ + i);
     }
-
-    ArgSlice ToSlice(CmdArgVec* scratch) const {
-      ToVec(scratch);
-      return *scratch;
-    }
-
-    void ToVec(CmdArgVec* vec) const {
-      vec->clear();
-      vec->reserve(args_->size() - index_);
-      for (auto arg : args_->view(index_))
-        vec->emplace_back(arg);
-    }
   };
 
   struct Slice : public ArgSlice {
@@ -189,14 +169,6 @@ class ParsedArgs {
     ParsedArgs Tail(unsigned offset = 1) const {
       return ParsedArgs{subspan(offset)};
     }
-
-    ArgSlice ToSlice(void* /*scratch*/) const {
-      return *this;
-    }
-
-    void ToVec(CmdArgVec* vec) const {
-      vec->assign(begin(), end());
-    }
   };
   std::variant<Slice, WrapperBacked> args_;
 };

src/server/command_registry.h

@@ -115,7 +115,7 @@ class CommandId : public facade::CommandId {
                          std::optional<facade::ErrorReply>(const facade::ParsedArgs&) const>;
 
   // Returns the invoke time in usec.
-  void Invoke(CmdArgList args, CommandContext* cmd_cntx) const {
+  void Invoke(const facade::ParsedArgs& args, CommandContext* cmd_cntx) const {
     handler_(facade::CmdArgParser{args}, cmd_cntx);
   }
 
@@ -148,10 +148,10 @@ class CommandId : public facade::CommandId {
     return interleave_step_;
   }
 
+  // Adapts a free-function handler (including coroutine handlers returning cmd::CmdR) into the
+  // void Handler, discarding the unused return object.
   template <typename RT> CommandId&& SetHandler(RT f(facade::CmdArgParser, CommandContext*)) && {
-    handler_ = [f](facade::CmdArgParser parser, CommandContext* cntx) {
-      f(std::move(parser), cntx);
-    };
+    handler_ = [f](facade::CmdArgParser parser, CommandContext* cntx) { f(parser, cntx); };
     return std::move(*this);
   }
 
@@ -161,9 +161,7 @@ class CommandId : public facade::CommandId {
     return std::move(*this).SetHandler(f);
   }
 
-  CommandId&& SetHandler(Handler f, bool async_support = false) && {
-    if (async_support)
-      kind_mask_ |= SUPPORT_ASYNC;
+  CommandId&& SetHandler(Handler f) && {
     handler_ = std::move(f);
     return std::move(*this);
   }

src/server/conn_context.cc

@@ -59,7 +59,7 @@ vector<string> FormatEvalSlowlog(const ConnectionState& state) {
 
 }  // namespace
 
-StoredCmd::StoredCmd(const CommandId* cid, facade::ArgSlice args, facade::ReplyMode mode)
+StoredCmd::StoredCmd(const CommandId* cid, const facade::ParsedArgs& args, facade::ReplyMode mode)
     : cid_{cid}, reply_mode_{mode} {
   backed_ = std::make_unique<cmn::BackedArguments>(args.begin(), args.end(), args.size());
   args_ = facade::ParsedArgs{*backed_};
@@ -73,10 +73,6 @@ StoredCmd::StoredCmd(const CommandId* cid, cmn::BackedArguments* src, uint8_t ta
   args_ = facade::ParsedArgs{*backed_, tail_index};
 }
 
-CmdArgList StoredCmd::Slice(CmdArgVec* scratch) const {
-  return args_.ToSlice(scratch);
-}
-
 std::string StoredCmd::FirstArg() const {
   if (NumArgs() == 0) {
     return {};
@@ -313,11 +309,10 @@ void CommandContext::ReuseInternal() {
   cid_ = nullptr;
   tx_ = nullptr;
   tail_args_ = {};
-  arg_slice_backing.clear();
   start_time_usec = 0;
 }
 
-void CommandContext::RecordLatency(facade::ArgSlice tail_args) const {
+void CommandContext::RecordLatency(const facade::ParsedArgs& tail_args) const {
   DCHECK_GT(start_time_usec, 0u);
   int64_t after = base::CycleClock::ToUsec(base::CycleClock::Now());
 
@@ -344,23 +339,26 @@ void CommandContext::RecordLatency(facade::ArgSlice tail_args) const {
 
   vector<string> aux_params;
   CmdArgVec aux_slice;
+  facade::ParsedArgs slowlog_args = tail_args;
 
   // Rewrite arguments for exec/eval with stats
   if (cid_->IsMultiTransactional()) {
     if (cid_->IsExec())
       aux_params = FormatExecSlowlog(cntx->conn_state);
     else if (cid_->IsEvalGroup())
       aux_params = FormatEvalSlowlog(cntx->conn_state);
+
     aux_slice = {aux_params.begin(), aux_params.end()};
     if (tail_args.size() > 0) {
+      facade::ParsedArgs args_suffix = tail_args;
       if (!aux_params.empty())
-        tail_args.remove_prefix(1);  // remove script/sha from eval/evalsha
-      aux_slice.insert(aux_slice.end(), tail_args.begin(), tail_args.end());
+        args_suffix = args_suffix.Tail(1);  // remove script/sha from eval/evalsha
+      aux_slice.insert(aux_slice.end(), args_suffix.begin(), args_suffix.end());
     }
-    tail_args = aux_slice;
+    slowlog_args = facade::ArgSlice{aux_slice};
   }
 
-  ServerState::SafeTLocal()->GetSlowLog().Add(cid_->name(), tail_args, conn->GetName(),
+  ServerState::SafeTLocal()->GetSlowLog().Add(cid_->name(), slowlog_args, conn->GetName(),
                                               conn->RemoteEndpointStr(), execution_time_usec,
                                               absl::GetCurrentTimeNanos() / 1000);
 }

src/server/conn_context.h

@@ -32,18 +32,15 @@ struct CmdRef {
   bool IsValid() const {
     return cid != nullptr;
   }
-
-  facade::ArgSlice Slice(CmdArgVec* scratch) const {
-    return args.ToSlice(scratch);
-  }
 };
 
 // Stores command id and arguments for delayed invocation.
 // Used for storing MULTI/EXEC commands.
 class StoredCmd {
  public:
   // Deep copy of args, creates backing storage internally.
-  StoredCmd(const CommandId* cid, ArgSlice args, facade::ReplyMode mode = facade::ReplyMode::FULL);
+  StoredCmd(const CommandId* cid, const facade::ParsedArgs& args,
+            facade::ReplyMode mode = facade::ReplyMode::FULL);
 
   // Moves args from src via swap. src's BackedArguments will be empty after this.
   // tail_index specifies how many leading args to skip (e.g., 1 to skip the command name).
@@ -58,7 +55,6 @@ class StoredCmd {
     return backed_ ? backed_->HeapMemory() + sizeof(*backed_) : 0;
   }
 
-  facade::ArgSlice Slice(CmdArgVec* scratch) const;
   const facade::ParsedArgs& Args() const {
     return args_;
   }
@@ -419,7 +415,7 @@ class CommandContext : public facade::ParsedCommand {
     return static_cast<ConnectionContext*>(conn_cntx_);
   }
 
-  void RecordLatency(facade::ArgSlice tail_args) const;
+  void RecordLatency(const facade::ParsedArgs& tail_args) const;
 
   facade::Connection* conn() const {
     return conn_cntx_->conn();
@@ -447,15 +443,11 @@ class CommandContext : public facade::ParsedCommand {
 
   uint64_t start_time_usec = 0;
 
-  // Stores backing array for tail args slice
-  CmdArgVec arg_slice_backing;
-
  protected:
   void ReuseInternal() final;
 
-  // Command arguments without the command name. Replaces arg_slice_backing as the
-  // canonical arg source once all handlers are migrated. Points into BackedArguments
-  // owned by this CommandContext (or StoredCmd for EXEC), so it survives async execution.
+  // Command arguments without the command name. Points into BackedArguments owned by this
+  // CommandContext (or StoredCmd for EXEC), so it survives async execution.
   facade::ParsedArgs tail_args_;
 
   Transaction* tx_ = nullptr;