Skip to content

perf(tui): トレース無効時にキー毎の TraceEvent 構築を省く#349

Merged
kkyosuke merged 1 commit into
mainfrom
review
Jun 25, 2026
Merged

perf(tui): トレース無効時にキー毎の TraceEvent 構築を省く#349
kkyosuke merged 1 commit into
mainfrom
review

Conversation

@kkyosuke

Copy link
Copy Markdown
Owner

目的

ホーム画面のイベントループは毎キー押下で操作トレースを記録しているが、トレース無効(既定)でもイベント構築のコストを払っていた。trace_log の docstring が掲げる「opt-in なのでホットパスのコストはゼロ」という不変条件を、この呼び出し箇所で実体に合わせる。

背景

event_loop のキー処理は次を呼んでいた:

TraceLog::record(
    TraceEvent::now(TraceCategory::Tui, "key")
        .with_detail(format!("{:?} {:?}", state.mode(), key)),
);

record() は内部で is_enabled() を見て即 return するが、引数TraceEvent::nowUtc::now() + 文字列確保)と format! は無効時でも先行評価される。キー入力は人間速度なので CPU 実害は小さいが、宣言された不変条件の破れであり修正は明快。

変更内容

  • infrastructure/trace_log.rs
    • TraceLog::record_with(impl FnOnce() -> TraceEvent) を追加。is_enabled() を通過してからクロージャでイベントを構築するため、無効時はタイムスタンプ・確保・format! のいずれも走らない。
    • record の本体を private な emit(&TraceEvent) に切り出し、record / record_with で共有。
    • 冷たい経路(CLI 終了・セッション create/remove・MCP は detail が "ok"/"error" と軽量)は record のまま据え置く。
  • presentation/tui/home/event/mod.rs
    • キー押下トレースを record_with(|| ..) に変更。

テスト・確認方法

  • cargo fmt / cargo clippy --all-targets -- -D warnings / cargo test 通過。
  • 追加テスト:
    • trace_log: record_with の有効/無効両分岐(無効時は書き込みなし、有効時は tui イベントが残る)。
    • event: USAGI_TRACE を有効にしてループを回し、キー押下が記録されることを確認(計測対象 event/mod.rs のクロージャをカバー)。
  • cargo llvm-cov --workspace(CI 相当)で Lines/Functions 100% を維持(変更ファイルともに 100%)。

補足(今回見送った性能観点の指摘)

レビューでは他に 2 点を挙げたが、いずれも変更を見送った:

  1. MonitorHandle::snapshot() の毎ティック clone — イベントループ側では結果が apply_badges に消費されるため clone は実質必須。没入側の非描画 wake での再計算も頻度が低い。回避には監視状態の世代カウンタ導入が要るが、live 集合は各ペインの生存フラグから毎フレーム導出しており世代化するとライブ判定が最大 200ms 遅延する挙動退行になる。便益も計測下では無で、見送り。
  2. 没入時のグリッド全文字列化 — 不変 Arc スナップショット設計の固有コストで、60fps コアレス+行差分で実画面書き込みは最小化済み。「修正」は設計の作り替えで退行リスクがあり便益も不確実。

どちらも欠陥ではなく、現状が妥当という判断。世代化アプローチを試す価値があると判断される場合は別途ご相談ください。

🤖 Generated with Claude Code

ホーム画面のイベントループは毎キー押下で
`TraceLog::record(TraceEvent::now(..).with_detail(format!("{mode} {key}")))`
を呼んでおり、引数の `TraceEvent`(`Utc::now()`・文字列確保)と `format!`
がトレース無効(既定)でも先行評価されていた。trace_log の docstring が
掲げる「opt-in なのでホットパスのコストはゼロ」という不変条件が、この
呼び出し箇所では成立していなかった。

`TraceLog::record_with(impl FnOnce() -> TraceEvent)` を追加し、`is_enabled()`
を通過してからイベントを構築するようにする。実際にホットなキー押下経路
だけをこれに移し、冷たい経路(CLI 終了・セッション create/remove・MCP は
detail が "ok"/"error" と軽量)は `record` のまま据え置く。

- trace_log: `record` の本体を `emit` に切り出し `record_with` と共有。
  両分岐+クロージャ本体を直接テスト。
- event: キー押下トレースを `record_with(|| ..)` に変更。トレース有効時に
  クロージャを通すテストを追加し、計測対象ファイルのカバレッジ 100% を維持。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📊 Test Coverage

🚀 いまのカバレッジ (Lines): 100.00% — この調子でいこう!

🎉✨ パーフェクト!全ファイル Lines カバレッジ 100% を達成しました 🏆🐰

@kkyosuke kkyosuke merged commit 59f3f61 into main Jun 25, 2026
3 checks passed
@kkyosuke kkyosuke deleted the review branch June 25, 2026 23:40
kkyosuke added a commit that referenced this pull request Jun 25, 2026
## 目的

没入(Attached)の描画は shell 出力に応じ最大 60fps で回る最ホットな経路。その 1 フレームあたりで
`term.size()`(TIOCGWINSZ ioctl)を**2 回**読んでいた冗長を解消する。

## 背景

`drive` ループは各イテレーションの先頭で `term.size()` を読んでペイン geometry を組み立て、続く
`render()` の中でも `term.size()`
を再度読んでフレーム全体のサイズにしていた。同一イテレーション内の連続呼び出しでターミナルサイズは変わり得ないので、2 回目は純粋に冗長な
syscall。出力フラッド時は毎描画フレームで余計な ioctl を 1 回払っていた。

## 変更内容

- `presentation/tui/home/terminal_pane.rs`
- ループ先頭で読んだ `(height, width)` を `render()` に引数で渡し、`render()` 内の
`term.size()` 再読を廃止。

## テスト・確認方法

- `cargo fmt` / `cargo clippy --all-targets -- -D warnings` / `cargo
test` 通過(pre-push フックでも clippy/test green)。
- `terminal_pane.rs` は端末 I/O
専用でカバレッジ計測の除外対象(`scripts/coverage.sh`)のため、カバレッジには影響しない。挙動は不変(同一フレーム内で同じサイズを使うだけ)。

## 補足

これは
[#349](https://github.com/kkyosuke/usagi/pull/349)(トレース無効時のキー毎構築を省く)と同じ「ホーム画面ホットパスの無駄削り」シリーズの一環。`#349`
がマージ済みのため、本変更は独立 PR として切り出した。

レビューで挙げた他の候補(`MonitorHandle::snapshot()` の毎ティック clone、没入のグリッド全文字列化、issue
書き込みの read_dir 重複、`issue/stats.rs::group()` の潜在
O(n²))は、いずれも*欠陥ではなく*、実規模では計測可能な改善にならない/挙動退行や複雑化のリスクが便益を上回ると判断し見送り。詳細は会話ログ参照。

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: test <test@example.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant