Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why iterate process when calling heim_process::get #333

Open
roblabla opened this issue May 27, 2021 · 2 comments
Open

Why iterate process when calling heim_process::get #333

roblabla opened this issue May 27, 2021 · 2 comments

Comments

@roblabla
Copy link
Contributor

On windows, heim_process::get(pid) will iterate on all pids to check if the pid exists:

This ends up being a significant performance bottleneck in my application in certain scenarios - epsecially if I try to get information about processes that may already be dead.

To give a bit more info: I get notified when the process start, and try to get the CWD as fast as possible. This is inherently racy and sometimes the process will die before I get the chance to acquire it (it happens often for short-lived processes like ls in bash for windows). When this happens, Heim will sometimes end up enumerating the pids, which slows down my app and makes it even less likely I'll get notified in time of future processes.

In practice, I don't quite understand the logic behind this enumeration. In my mind, we can do this two ways:

  • Since we have an exit code, that means the process already died and checking the pids is just extra work that doesn't actually tell us much.
  • In addition, actually checking the exit code doesn't actually feel super useful: If we managed to acquire a ProcessHandle, then the process is still available for querying information. Maybe we shouldn't error out at all, even if the process is already dead, so long as we can acquire a ProcessHandle for it?
@svartalf
Copy link
Member

Hey, @roblabla!

It was done to provide better guarantees for pid_exists function. Since current implementation is pretty much a copy of psutil (Python one) library, you can find more details on it here: giampaolo/psutil#1094

@roblabla
Copy link
Contributor Author

PSUtil doesn't call pid_exists when getting a process by pid though. E.G. in psutil, the following:

process = Process(512)
cwd = process.cwd()

Will never call pid_exists as far as I can tell. pid_exists is only called in the wait function, and in the send_signal function, but only for OpenBSD.

In heim, this equivalent code will call pid_exists (and thus may call pids):

let process = heim_process::get(512);
let cwd = process.cwd()

This feels unnecessary. It doesn't actually improve reliability since the process may die between the get and the cwd functions, and cwd (and all other functions on Process) already handle the case where the process dies on us by returning an error.

When the user tries to get a Process, we shouldn't check if the pid exists (beyond raising an error if getting the create_time fails) IMO.

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

No branches or pull requests

2 participants