From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 14 06:55:55 2021 Received: (at 33848) by debbugs.gnu.org; 14 Apr 2021 10:55:55 +0000 Received: from localhost ([127.0.0.1]:33692 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lWdBa-0001XD-Lu for submit@debbugs.gnu.org; Wed, 14 Apr 2021 06:55:54 -0400 Received: from eggs.gnu.org ([209.51.188.92]:47764) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lWdBY-0001Wy-Mb for 33848@debbugs.gnu.org; Wed, 14 Apr 2021 06:55:53 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:40213) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lWdBS-0003j8-1W; Wed, 14 Apr 2021 06:55:46 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=34842 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lWdBQ-0005yR-Qr; Wed, 14 Apr 2021 06:55:45 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Mark H Weaver Subject: Re: bug#33848: Store references in SBCL-compiled code are "invisible" References: <87r2e8jpfx.fsf@gnu.org> <87k1juaomo.fsf@gnu.org> <87muoqhk62.fsf@ambrevar.xyz> <87zhsq8wkj.fsf@gnu.org> <87d0pmhbgn.fsf@ambrevar.xyz> <87r2e28tkv.fsf@gnu.org> <874laygkiy.fsf@ambrevar.xyz> <87lfa5eymf.fsf@ambrevar.xyz> <87tuoscsk9.fsf@gnu.org> <87im57b8u7.fsf@ambrevar.xyz> <87czvebky2.fsf@netris.org> <87eefu30a4.fsf@gnu.org> <87im56l6es.fsf@yamatai> <87wntm8j18.fsf@ambrevar.xyz> <87a6qil4b1.fsf@yamatai> <87a6qiz5b3.fsf@ambrevar.xyz> <871rbtc3j5.fsf@netris.org> <87r1js9udv.fsf@netris.org> <87h7kj7j7x.fsf@netris.org> <87ft01axta.fsf@gnu.org> <87k0p6rlt5.fsf@netris.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 25 Germinal an 229 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Wed, 14 Apr 2021 12:55:43 +0200 In-Reply-To: <87k0p6rlt5.fsf@netris.org> (Mark H. Weaver's message of "Tue, 13 Apr 2021 16:06:19 -0400") Message-ID: <87eefdf840.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 33848 Cc: Guillaume Le Vaillant , Pierre Neidhardt , 33848@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.7 (-) Hi Mark, Mark H Weaver skribis: >> The risks of bugs I can think of are: missed ASCII references (a >> regression, whereby some ASCII references would not get rewritten), > > This is indeed a risk whenever we modify the grafting code, which is > written for efficiency, not clarity. I've tried to be careful, and have > checked that my newly grafted system and user profiles do not retain > references to ungrafted code, modulo the following pre-existing issues: > > > (ibus-daemon launches ungrafted subprocesses) > > > (Chunked store references in .zo files in Racket 8) OK. >> and unrelated UTF-{16,32}-base32-looking references getting rewritten. >> >> I guess the latter is very unlikely because only sequences found in the >> replacement table may be rewritten, right? > > Yes, that's correct. > >> The former should be caught by =E2=80=98tests/grafts.scm=E2=80=99 but we= could also >> check the closure of real-world systems, for instance, to make sure it >> doesn=E2=80=99t refer to ungrafted things. > > As I mention above, I've already done so for my own GNOME-based Guix > system. Yes, we should be safe. >> Do you know how this affects performance? > > I have not yet measured it, but subjectively, I do not notice any > obvious difference in speed. > > I expect that the main performance impact is that large blocks of NULs > will be processed more slowly by the current draft version of the new > grafting code. That's because NULs can now be part of a nix hash, and > therefore the new grafting code can only advance 1 byte position when > seeing a NUL, whereas previously it would skip ahead 33 positions in > that case. > > If desired, the handling of NULs could be made more efficient, at the > cost of a bit more complexity. When seeing a NUL, we could check the > adjacent bytes to see if this is part of a nix-base32 character in > UTF-16 or UTF-32 encoding. If not, we could skip ahead. Let=E2=80=99s keep it this way for now; we can always revisit later if we f= eel the need for it. >> For clarity, perhaps you can define a top-level procedure for the test >> and call it from =E2=80=98for-each=E2=80=99. > > Good idea. I'd like to optimize the tests a bit before pushing this. > They take a fairly long time to run, and lead to a huge 1.5G grafts.log > file. It might be sufficient to avoid 'test-equal' here. Ah yes, rather use =E2=80=98test-assert=E2=80=99 or some such to avoid the = huge log file. :-) > Below, I've attached my current revision of this draft patch, which > incorporates your suggestions regarding the main code. What remains is > to improve the tests. LGTM! Feel free to push this version or an improved one. I think it=E2=80= =99s good to have it in the upcoming release, and if it=E2=80=99s pushed sooner, we=E2=80=99ll have more time to react in case something=E2=80=99s wrong. Thank you! Ludo=E2=80=99.