Skip to content

DELETE-commit 5 protocol で remove_value() の Status 捨て分岐が重複 → MasstreeWrapper 側に void wrapper を追加して統一 #59

@thawk105

Description

@thawk105

発見経緯: #58 (Phase 4 -Wunused-variable 潰し) で、5 protocol の writePhase / commit の DELETE 分岐に 同じ shape のコードが重複 していて、subagent が (void) キャストを 5 箇所別個に追加するハメになった。スコープを守るためその PR では共通化せず、ここに切り出す。

現状

MasstreeWrapper::remove_value(key)Status (OK / WARN_NOT_FOUND) を返すが、DELETE commit の文脈では 「あれば消す、無くてもいい」セマンティクスで使われていて、Status を全 callsite で捨てている:

Protocol Callsite
silo cc/silo/transaction.cc:30, cc/silo/transaction.cc:557
mocc cc/mocc/transaction.cc:961, cc/mocc/transaction.cc:1050
tictoc cc/tictoc/transaction.cc:480, cc/tictoc/transaction.cc:529
ss2pl cc/ss2pl/transaction.cc:88
d2pl cc/d2pl/transaction.cc:88

うち #58(void) 明示が入ったのは 5 箇所、残り 3 箇所はもともと戻り値レス呼び出し。実質的な意味は同じ。

提案

MasstreeWrapper戻り値 void の「あれば消す」wrapper を追加し、上記 8 callsite を置き換える:

// include/masstree_wrapper.hh
// "delete if present" — discards "not found" since callers in the
// DELETE-commit path may race with concurrent deletions of the same key.
void remove_value_if_present(std::string_view key) {
  (void) remove_value(key);
}

各 protocol 側:

- (void)Masstrees[get_storage((*itr).storage_)].remove_value((*itr).key_);
+ Masstrees[get_storage((*itr).storage_)].remove_value_if_present((*itr).key_);

メリット

  • (void) キャストの boilerplate が消える
  • 「Status を返さない wrapper を使ってる」というシグナルが読み手に伝わる (= 意図的に捨てている、check すべき場面ではない)
  • 将来 -Werror=unused-result 系のフラグを有効化したり、Status[[nodiscard]] を付けたりした場合、ここで一斉に書き換えが要らなくなる
  • 新プロトコル追加時に「(void) の付け忘れ」事故を構造的に防げる

慎重に扱うべき点

ccbench は 研究ベンチマークなので各 protocol を独立に保つ のがコア設計思想。今回の提案は MasstreeWrapper 側 (= 全 protocol が依存する共通レイヤ) にだけ手を入れて、protocol-specific なロックタイミングや GC キュー push 順序などには一切触らない。それ以上の「writePhase の DELETE 分岐を共通ヘルパー化」のような踏み込んだ refactor は やらない

優先度

低 (cosmetic refactor)。Phase 5 (#43) で set_compile_options() を全 target に有効化するときの追加 cleanup として一緒に入れるか、独立 PR で気軽に潰す程度の話。

To-do

  • MasstreeWrapper::remove_value_if_present(std::string_view) を追加 (実装は remove_value()(void) で呼ぶだけ)
  • 8 callsite を置き換え
  • 5 protocol すべてで Release / Debug+ASan のビルドが通ることを確認

Related

  • 発見: #58 (Phase 4 -Wunused-variable 潰し)
  • #43 (phased -Werror cleanup) の周辺タスク

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions