-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
libutil, libexpr: #10542 abstract over getrusage for getting cpuTime stat and implement windows version #13767
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
base: master
Are you sure you want to change the base?
Conversation
a14edaa
to
e9ff717
Compare
Thanks for this! Yeah I assume it's fine to just throw now. And only if we see it failing in practice do we need to worry about adding code to catch the failure. |
Many thanks, @Ericson2314 and others for bearing with me and looking through this pretty minor change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking much better, thanks! I do have one comment though.
Head branch was pushed to by a user without write access
…Time stat and implement windows version Update src/libutil/windows/current-process.cc Prefer `nullptr` over `NULL` Co-authored-by: Sergei Zimmerman <[email protected]> Update src/libutil/unix/current-process.cc Prefer C++ type casts Co-authored-by: Sergei Zimmerman <[email protected]> Update src/libutil/windows/current-process.cc Prefer C++ type casts Co-authored-by: Sergei Zimmerman <[email protected]> Update src/libutil/unix/current-process.cc Don't allocate exception Co-authored-by: Sergei Zimmerman <[email protected]>
Motivation is to move towards Nix on Windows.
libexpr
usesgetrusage
from thesys/resource.h
Unix header to get the current process's running duration. This PR creates a new function inlibutil
which has both a Unix implementation and a new Windows implementation, and removes several conditional compile guards related to getting the cpu-time on Windows.The expected behavior is entirely unchanged because Nix doesn't target Windows yet.
The test suite is passing on my machine, but I did not do anything to test the change itself.