Skip to content

perf: avoid refresh full pid tree when Procs::new()#441

Merged
jdx merged 1 commit into
jdx:mainfrom
gaojunran:perf-proc-new
May 17, 2026
Merged

perf: avoid refresh full pid tree when Procs::new()#441
jdx merged 1 commit into
jdx:mainfrom
gaojunran:perf-proc-new

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

fixes #439.

@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR avoids a full sysinfo process-tree scan on Procs::new() and replaces the broad refresh_processes() calls scattered across the codebase with targeted refresh_pids(&[pid]) calls that only refresh the specific PIDs actually needed.

  • Procs::new() now returns an empty struct; every call site that needs process data explicitly refreshes the relevant PIDs before querying, removing the hidden startup cost.
  • A refresh_daemon_pids helper and a refresh_daemon_list_pids local helper are introduced to consolidate the common pattern of collecting daemon PIDs and refreshing only those.
  • Precondition documentation is added to kill and kill_process_group making the required refresh-before-call contract explicit; watchers.rs retains its refresh_processes() call because it needs the full process tree for child-process resource aggregation via get_batch_group_stats.

Confidence Score: 5/5

Safe to merge — all call sites correctly refresh targeted PIDs before querying; the one path that still needs a full refresh (watchers.rs) is untouched.

Every is_running, title(), and kill call site was audited and each has a refresh_pids immediately preceding it. The resource-monitoring watcher retains its full refresh_processes() because it uses get_batch_group_stats over the complete process tree. No silent no-ops or missed refreshes were found in the changed paths.

No files require special attention.

Important Files Changed

Filename Overview
src/procs.rs Core change: Procs::new() no longer pre-populates sysinfo; adds refresh_daemon_pids helper and precondition docs on kill/kill_process_group.
src/supervisor/lifecycle.rs Adds refresh_pids after daemon spawn (before upsert_daemon reads title()); replaces full refresh with targeted PID check in stop path.
src/supervisor/mod.rs Fixes implicit dependency on stale PROCS state in start_if_not_running and run; adds explicit targeted refresh before each is_running/title() call.
src/tui/app.rs TUI stats refresh now targets only daemon root PIDs instead of all processes; consistent with get_stats which only examines the root PID anyway.
src/web/routes/daemons.rs Extracts refresh_daemon_list_pids helper; replaces all broad refreshes with targeted ones; show now reads StateFile earlier to get the PID before refreshing.
src/cli/supervisor/mod.rs Adds refresh_pids(&[existing_pid]) before is_running in kill_or_stop, satisfying the new precondition contract.
src/cli/wait.rs Moves refresh to loop start and replaces broad refresh_processes with targeted refresh_pids; semantically equivalent and more efficient.
src/web/routes/index.rs Reads StateFile before calling refresh_daemon_pids so only managed daemon PIDs are refreshed on each page render.

Reviews (2): Last reviewed commit: "perf: avoid refresh full pid tree when P..." | Re-trigger Greptile

Comment thread src/web/routes/daemons.rs Outdated
@gaojunran gaojunran changed the title perf: avoid refresh full pid tree when PROC::new() perf: avoid refresh full pid tree when Procs::new() May 16, 2026
@jdx jdx merged commit ca42e0b into jdx:main May 17, 2026
5 checks passed
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.

2 participants