From: "Ludovic Courtès" <ludo@gnu.org>
To: Julien Lepiller <julien@lepiller.eu>
Cc: 39728@debbugs.gnu.org
Subject: [bug#39728] [PATCH] Allow parallel downloads and builds
Date: Mon, 24 Feb 2020 22:23:45 +0100 [thread overview]
Message-ID: <87h7zfel26.fsf@gnu.org> (raw)
In-Reply-To: <20200221235307.535fb453@tachikoma.lepiller.eu> (Julien Lepiller's message of "Fri, 21 Feb 2020 23:53:07 +0100")
Hi!
Julien Lepiller <julien@lepiller.eu> skribis:
> This patch allows to count builds and downloads separately. The idea is
> that downloads need bandwidth, but no CPU, while builds do not need
> bandwidth, but need CPU. With this patch, guix will be able to download
> substitutes while building unrelated packages. Currently, guix needs to
> wait for the download to finish before proceeding to the build. This
> should reduce the time of guix commands that need to build and download
> things at the same time.
>
> What do you think?
I think it’s a good idea!
I wonder what the UI will look like: (guix status) would no longer
display a progress bar when there’s more than on job (build or download)
taking place at the same time.
>>From 9c059d81ba4f4016f8c400b403f8c5edbdb160c2 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Fri, 21 Feb 2020 23:41:33 +0100
> Subject: [PATCH] nix: Count build and download jobs separately.
^
I’d write “daemon:” here. :-)
> This allows to run downloads (that take bandwith) and builds (that take
^
“This allows us”
> CPU time) independently from one another.
>
> * nix/nix-daemon/guix-daemon.cc: Add a max-download-jobs option.
> * nix/libstore/globals.hh: Add a maxDownloadJobs setting.
> * nix/libstore/globals.cc: Add a default value to it.
> * nix/libstore/build.cc: Manage build and download jobs separately.
For the final patch, please specify the entities changed (classes,
functions, etc.).
> + /* Number of download slots occupied. This includes substitution and
> + built-ins. */
> + unsigned int nrDownloads;
Note that not all builtins are downloads. Fixed-output derivations are
(usually) also downloads.
(It’d be the first time the daemon gets a notion of “download”. We
should make sure it doesn’t conflict with other assumptions.)
> /* Registers a running child process. `inBuildSlot' means that
> the process counts towards the jobs limit. */
> void childStarted(GoalPtr goal, pid_t pid,
> - const set<int> & fds, bool inBuildSlot, bool respectTimeouts);
> + const set<int> & fds, bool inBuildSlot, bool inDownloadSlot,
> + bool respectTimeouts);
How about replacing these two Booleans by a single enum?
> + unsigned int curDownloads = worker.getNrDownloads();
> + if (curDownloads >= (settings.maxDownloadJobs==0?1:settings.maxDownloadJobs) &&
> + fixedOutput) {
This is hard to parse and lacking spacing. :-) Perhaps make an
intermediate function or variable?
> +void Worker::waitForDownloadSlot(GoalPtr goal)
> +{
> + debug("wait for download slot");
> + if (getNrDownloads() < (settings.maxDownloadJobs==0?1:settings.maxDownloadJobs))
Same here.
> @@ -118,6 +119,7 @@ void Settings::update()
> {
> _get(tryFallback, "build-fallback");
> _get(maxBuildJobs, "build-max-jobs");
> + _get(maxDownloadJobs, "download-max-jobs");
We should also allow ‘set-build-options’ to set this option, as well as
add it to ‘%standard-build-options’. That can prolly come in a separate
patch.
> + { "max-downloads", 'D', n_("N"), 0,
> + n_("allow at most N download jobs") },
We’d need to update doc/guix.texi.
It would be great if you could test this patch for your daily usage. I
find it surprisingly easy to break things in the daemon. :-)
Thank you!
Ludo’.
next prev parent reply other threads:[~2020-02-24 21:24 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-21 22:53 [bug#39728] [PATCH] Allow parallel downloads and builds Julien Lepiller
2020-02-24 21:23 ` Ludovic Courtès [this message]
2020-02-25 15:21 ` zimoun
2020-02-25 15:39 ` Julien Lepiller
2020-02-26 10:36 ` Pierre Neidhardt
2021-11-21 22:49 ` [bug#39728] [PATCH v2] " Julien Lepiller
2021-11-25 12:53 ` Ludovic Courtès
2021-11-25 13:01 ` Julien Lepiller
2021-11-26 10:16 ` Ludovic Courtès
2021-07-14 2:49 ` [bug#39728] [PATCH] " Maxim Cournoyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h7zfel26.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=39728@debbugs.gnu.org \
--cc=julien@lepiller.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.