On 2025-01-16 at 10:18 +0100, Helmut Grohne wrote:
> Hi Julien,
>
> On Mon, Jan 13, 2025 at 07:00:01PM +0100, Julien Plissonneau Duquène
> wrote:
> > Let's start with this then. I implemented a PoC prototype [1] as a
> > shell
> > script that is currently fairly linux-specific and doesn't account
> > for
> > cgroup limits (yet?). Feedback is welcome (everything is open for
> > discussion
> > there, including the name) and if there is enough interest I may
> > end up
> > packaging it or submitting it to an existing collection (I am
> > thinking about
> > devscripts).
>
> I'm sorry for not having come back earlier and thus caused duplicaton
> of
> work. I had started a Python-based implementation last year and then
> dropped the ball over other stuff. It also implements the --require-
> mem
> flag in the way you suggested. It parses DEB_BUILD_OPTIONS,
> RPM_BUILD_NCPUS and CMAKE_BUILD_PARALLEL_LEVEL and also considers
> cgroup
> memory limits. I hope this captures all of the feedback I got during
> discussions and research.
>
> I'm attaching my proof of concept. Would you join forces and turn
> either
> of these PoCs into a proper Debian package that could be used during
> package builds? Once accepted, we may send patches to individual
> Debian
> packages making use of it and call OOM FTBFS a packaging bug
> eventually.
>
> Helmut
The script looks good, and easy to read. It wouldn't be hard to
translate it to another language if needed to drop the python
dependency (but that would increase the nloc)
I find this behavior a bit surprising:
$ python3 guess_concurrency.py --min 10 --max 2
10
If there is a minimum limit, it is returned, even if that violates the
max. It makes some sense to pick something but I as actually expecting
an error to the above.
The order of processing the cpus is a bit awkward as well.
The order it uses is CMAKE_BUILD_PARALLEL_LEVEL, DEB_BUILD_OPTIONS,
RPM_BUILD_NCPUS, --detect <n>, nproc/os.cpu_count()
But the order in the code is 4, 5, 3, 2, 1
Not straightforward.
Also, it is doing actions such as running external program nproc even
it if's going to be discarded later. (nproc is in an essential package,
I know, but still)
Also, why would the user want to manually specify between nptoc and
os.cpu_count()?
I would unconditionally call nproc, with a fallback to os.cpu_count()
if that fails (I'm assuming nproc may be smarter than os.cpu_count(),
otherwise one could use cpu_count() always)
I suggest doing:
def main() -> None:
parser = argparse.ArgumentParser()
parser.add_argument(
"--count",
action="store",
default=None,
metavar="NUMBER",
help="supply a processor count",
)
(...)
args = parser.parse_args()
guess = None
try:
if args.count:
guess = positive_integer(args.count)
except ValueError:
parser.error("invalid argument to --count")
guess = guess or guess_from_environment("CMAKE_BUILD_PARALLEL_LEVEL")
guess = guess or guess_deb_build_parallel()
guess = guess or guess_from_environment("RPM_BUILD_NCPUS")
if not guess:
try:
guess = guess_nproc()
finally:
guess = guess or guess_python()
Additionally, the --ignore argument of nproc(1) might be of use for
this script as well.
Best regards
Ángel
