From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 15 04:56:09 2022 Received: (at 59078) by debbugs.gnu.org; 15 Nov 2022 09:56:09 +0000 Received: from localhost ([127.0.0.1]:52998 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ousfo-0002TC-J9 for submit@debbugs.gnu.org; Tue, 15 Nov 2022 04:56:09 -0500 Received: from mira.cbaines.net ([212.71.252.8]:41836) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ousfi-0002Sy-U1 for 59078@debbugs.gnu.org; Tue, 15 Nov 2022 04:56:06 -0500 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:54d1:d5d4:280e:f699]) by mira.cbaines.net (Postfix) with ESMTPSA id 6472327BBE9; Tue, 15 Nov 2022 09:56:01 +0000 (GMT) Received: from felis (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id b72db4c8; Tue, 15 Nov 2022 09:55:59 +0000 (UTC) References: <20221106135532.5724-1-mail@cbaines.net> <87k041dui7.fsf@gnu.org> <87bkpau4kl.fsf@cbaines.net> <87h6z1puli.fsf@gnu.org> User-agent: mu4e 1.8.11; emacs 28.2 From: Christopher Baines To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#59078: [PATCH] lint: Split the derivation lint checker by system. Date: Tue, 15 Nov 2022 08:35:42 +0000 In-reply-to: <87h6z1puli.fsf@gnu.org> Message-ID: <87leocfsnm.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 59078 Cc: 59078@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Hi! > > Christopher Baines skribis: > >> Ludovic Court=C3=A8s writes: > > [...] > >>> The =E2=80=98derivation=E2=80=99 checker was added for this purpose: ma= king sure that a >>> package=E2=80=99s derivation can be computed for all the supported syst= ems. >>> Previously it was easy to overlook that kind of breakage. >>> >>> I think it=E2=80=99s important to keep a =E2=80=98derivation=E2=80=99 c= hecker that does this. >> >> What aspect of it do you think is important? > > I meant that it=E2=80=99s important to have a single =E2=80=98derivation= =E2=80=99 checker that > checks derivations for all the supported systems. Packagers should be > able to run =E2=80=98guix lint -c derivation PKG=E2=80=99 and be confiden= t that it=E2=80=99s > fine for all systems. I think we can still keep that by adding support for grouping lint checkers. So have a derivation group, instead of a single checker. Maybe that'll change the command slightly to 'guix lint -g derivation PKG', but I think that can be equivalent. >>> Now, the memory consumption you report is unacceptable and this needs to >>> be addressed. >>> >>> Most (all?) caches are now per-session (associated with >>> ). Since =E2=80=98guix lint=E2=80=99 uses a single s= ession, those >>> caches keep growing because there=E2=80=99s no eviction mechanism in pl= ace. >>> >>> A hack like the one below should work around that. Could you check how >>> well it works for you? >> >> I tried in the Guix Data Service processing packages in chunks of 1000 >> plus closing the store connection after each batch, > > How was it implemented? Was it after the caches came into > ? I'm not sure what aspect of the implementation is important, but I think it's working correctly. Closing the store connection wasn't very easy. Previously there was a fresh store connection with each call to inferior-eval-with-store, but for this test I close the connections in %store-table and then clear the hash table between the inferior-eval-with-store calls. >> and that led to a heap size of 3090MiB. But this is still higher than >> 1778MiB heap usage I got just by splitting the derivation linter. > > I didn=E2=80=99t take the time to do it, but it would be nice to see, wit= h the > patch I gave, how =E2=80=98guix lint -c derivations=E2=80=99 behaves. I've put some numbers below, with no changes the last batch to finish processing leaves the heap at 7755MiB [1], then Guile crashes after that. With the patch you sent, the heap size seems to stabilise at 4042MiB [2]. It also crashes at the end due to the match block not matching '(), but that's not important. I also hacked the lint script to run the checkers in the same manor as the Guix Data Service, so one checker at a time rather than one package at a time. That led to a max heap size of 3505MiB [3]. By adding in batching (as the Guix Data Service already does), I think it's possible to further reduce this to the 1778MiB number I give above. Reducing the memory usage helps reduce the cost/improve the throughput of loading data in to the Guix Data Service which is my primary motivation here. I'm also not only concerned with reducing the peak memory usage, but trying to have an implementation that'll gracefully handle more systems being supported in the future. It's for that second point that I think arranging the derivation linting so that it's possible to process each system in turn is important for the Guix Data Service, so that when new platforms are added, the memory usage won't grow as much. 1: no batching, one derivation checker 1065.0 MiB 1409.0 MiB 2089.0 MiB 2297.0 MiB 2513.0 MiB 2705.0 MiB 3077.0 MiB 3373.0 MiB 3557.0 MiB 3661.0 MiB 3901.0 MiB 3997.0 MiB 4147.0 MiB 4491.0 MiB 4635.0 MiB 5899.0 MiB 7755.0 MiB 2: batches of 1000 with fresh store connection, one derivation checker 1057.0 MiB 1137.0 MiB 1481.0 MiB 1481.0 MiB 1481.0 MiB 1481.0 MiB 1697.0 MiB 1697.0 MiB 1697.0 MiB 1761.0 MiB 1761.0 MiB 1761.0 MiB 1937.0 MiB 1977.0 MiB 1985.0 MiB 3065.0 MiB 3633.0 MiB 4041.0 MiB 4042.0 MiB 4042.0 MiB 4042.0 MiB 3: multiple derivation checkers, batched by system 1297.0 MiB 1545.0 MiB 1873.0 MiB 2113.0 MiB 2353.0 MiB 2609.0 MiB 2961.0 MiB 3193.0 MiB 3505.0 MiB --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmNzYi1fFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9Xf2Tg/+MEKPBt1Ibu6sWQD9Xia/nR7MTimBHcOw dA8/WR13He3MwjxwaBjSI+T09RVYokukCYbpf4UjkxgU3np+bDyAUj7XA/C01m6b 67jYpuZ0+11Jgm4iLbZWh8O7YVX+K8hilnLynFTEU4X5ns0way4L1pGrN19LmnPt dS0Dh7Ul3j5rCHMMWjCrrK9/p51snm12lZCV1uk9bvhYrkoEC/ByO/1T9o3FS7h7 WD3dmBN3eqwtfe4VsPVBf+6XREBDlxDTBPlxsGd+RsmghFkR5k34WQP5otyAP+R7 CpeKZ2K/BoU1CJduz5fMQQUtAumASGBS+symysyIUyRIp2hcZyHTp4YbvbkjxwIe 9CS/Oq/qIQFoOpFfX8qGb/23EJc761YnO+73Pjk42ggWwO+nl45tN20T4jwUTblJ 7L+RoKBhK+pdn/ojJfFdk1CoSnwM0sihkhkuEAaNJ6lDr8wYSb2q5O73ZzKoBCUT lw1PrPDNEv3D2BQ1w1mO1dste6X0Jxb7ZbIGUJRGLkU+p4+RII3N59/EXceiq69w WOaFaYvo/Cij5y2ZPGocIgZofHaFImd3A1oBoa3YgaAA2hopQqugbDBLz6WLnVMR +gIMgzd0UhHoPGIXI3ZsQkGsYW3eA1sY++hBxzkciBMG4n/+etsfk9SJswWz7je9 QpTiSyazp4M= =m6Vq -----END PGP SIGNATURE----- --=-=-=--