From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:57359) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j6LCt-0003NO-ML for guix-patches@gnu.org; Mon, 24 Feb 2020 16:24:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j6LCs-0008BG-Fe for guix-patches@gnu.org; Mon, 24 Feb 2020 16:24:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:48231) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1j6LCs-0008Az-BR for guix-patches@gnu.org; Mon, 24 Feb 2020 16:24:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1j6LCs-0005mm-8a for guix-patches@gnu.org; Mon, 24 Feb 2020 16:24:02 -0500 Subject: [bug#39728] [PATCH] Allow parallel downloads and builds Resent-Message-ID: From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20200221235307.535fb453@tachikoma.lepiller.eu> Date: Mon, 24 Feb 2020 22:23:45 +0100 In-Reply-To: <20200221235307.535fb453@tachikoma.lepiller.eu> (Julien Lepiller's message of "Fri, 21 Feb 2020 23:53:07 +0100") Message-ID: <87h7zfel26.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Julien Lepiller Cc: 39728@debbugs.gnu.org Hi! Julien Lepiller 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=E2=80=99s a good idea! I wonder what the UI will look like: (guix status) would no longer display a progress bar when there=E2=80=99s more than on job (build or down= load) taking place at the same time. >>>From 9c059d81ba4f4016f8c400b403f8c5edbdb160c2 Mon Sep 17 00:00:00 2001 > From: Julien Lepiller > Date: Fri, 21 Feb 2020 23:41:33 +0100 > Subject: [PATCH] nix: Count build and download jobs separately. ^ I=E2=80=99d write =E2=80=9Cdaemon:=E2=80=9D here. :-) > This allows to run downloads (that take bandwith) and builds (that take ^ =E2=80=9CThis allows us=E2=80=9D > 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=E2=80=99d be the first time the daemon gets a notion of =E2=80=9Cdownlo= ad=E2=80=9D. We should make sure it doesn=E2=80=99t 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 & fds, bool inBuildSlot, bool respectTimeouts); > + const set & fds, bool inBuildSlot, bool inDownloadSlot, > + bool respectTimeouts); How about replacing these two Booleans by a single enum? > + unsigned int curDownloads =3D worker.getNrDownloads(); > + if (curDownloads >=3D (settings.maxDownloadJobs=3D=3D0?1:settings.ma= xDownloadJobs) && > + 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=3D=3D0?1:settings.m= axDownloadJobs)) 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 =E2=80=98set-build-options=E2=80=99 to set this option= , as well as add it to =E2=80=98%standard-build-options=E2=80=99. That can prolly come = in a separate patch. > + { "max-downloads", 'D', n_("N"), 0, > + n_("allow at most N download jobs") }, We=E2=80=99d 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=E2=80=99.