search

michaelforney / samurai

ninja-compatible build tool written in C
Other
832 stars 47 forks source link

Load average option #82

Closed capezotte closed 2 years ago

capezotte commented 2 years ago

This is an alternative implementation of #41, and owes quite a bit of code to #50.

This avoids having multiple threads and instead relies on making poll time out.

I'm not sure of the best way to guard the load-average-using code, though. There doesn't seem to be a macro associated with it (and forgive me if my lack of experience with C is showing). That code is still unguarded in this PR.

capezotte commented 2 years ago

Once we get these initial issues sorted out, I'd like to see what options would be best for guarding this code.

capezotte commented 2 years ago

I'm thinking of guarding this commit's additions with #ifdef HAS_GETLOADAVG and documenting the need for adding -DHAS_GETLOADAVG somewhere. Does this sound acceptable to you?

michaelforney commented 2 years ago

I'm thinking of guarding this commit's additions with #ifdef HAS_GETLOADAVG and documenting the need for adding -DHAS_GETLOADAVG somewhere. Does this sound acceptable to you?

Since this API is pretty widely available, I think we could do the opposite and introduce a NO_GETLOADAVG to disable its use.

(sorry for the late reply, I've been a bit busy lately)

capezotte commented 2 years ago

Guards are in place now.

capezotte commented 2 years ago

I'm satisfied with the state of this PR.

orbea commented 2 years ago

@michaelforney What's the status of this PR?

michaelforney commented 2 years ago

Pushed with a couple of tweaks.

Thanks for working on this, and sorry it took me so long to merge.

michaelforney commented 2 years ago

@capezotte Any idea if macOS has getloadavg, and if so, how to expose it?

Currently it is failing due to missing declaration: https://github.com/michaelforney/samurai/runs/7437461267

I'm not quite sure if we should set -DNO_GETLOADAVG for the mac build, or if some other define or include is needed for it.

capezotte commented 2 years ago

I looked into Apple's stdlib.h and it seems to want us to #define _DARWIN_C_SOURCE to get access to getloadavg.

By the way, I checked the Ubuntu build and glibc prints a warning for defining _BSD_SOURCE.

So it should look like this:

/* expose getloadavg */
#ifndef NO_GETLOADAVG
#   if defined(__APPLE__) && defined(__MACH__)
#       define _DARWIN_C_SOURCE
#   else
#       define _BSD_SOURCE
#       define _DEFAULT_SOURCE /* silence glibc >= 2.19 warning */
#   endif
#endif
orbea commented 2 years ago

@capezotte I added it to my warning fix PR, thanks for looking.

https://github.com/michaelforney/samurai/pull/87