Skip to content

perf: only probe node processes when looking for stale managers - #57

Open
donbowman wants to merge 1 commit into
wey-gu:mainfrom
donbowman:perf/kill-existing-processes
Open

donbowman wants to merge 1 commit into
wey-gu:mainfrom
donbowman:perf/kill-existing-processes

Conversation

@donbowman

Copy link
Copy Markdown

Summary

_kill_existing_processes runs on every PGliteManager.start(). It used psutil.process_iter(["pid", "name", "cmdline", "cwd"]) and inspected every process on the host; process_iter fetches the requested attributes eagerly, so on a busy machine (~2300 processes) it costs ~150ms per start even though nothing matches.

  • Only processes whose name looks like node can be running pglite_manager.js. Filter on name first and fetch cmdline/cwd lazily for those few processes (measured ~98ms on the same host, with only a couple of processes actually probed).
  • Skip the scan entirely when the work directory was auto-created: tempfile.mkdtemp() is brand new, so no process can have it as its working directory.

Semantics for a reused work directory are unchanged: a node process running pglite_manager.js with cwd equal to the work directory is still killed.

Testing

  • Updated tests/test_manager_comprehensive.py for the lazy probing, and added cases: an auto-created work dir skips the scan, unrelated node processes are ignored, and unreadable process names are handled safely.
  • pytest tests/test_manager_comprehensive.py tests/test_enhanced_process_cleanup.py -m "not slow" → 59 passed.

_kill_existing_processes scanned every process on the machine with
psutil and pulled cmdline and cwd for each, which costs ~100-150ms on a
busy host even though only node processes can be running
pglite_manager.js.  Filter on the process name first and fetch
cmdline/cwd lazily for the few node processes, and skip the scan
entirely when the work directory was auto-created (a brand new temp dir
cannot have a stale process).

This branch has not been deployed

No deployments
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