GNU bug report logs - #36447
27.0.50; New "Unknown keyword" errors

Previous Next

Package: emacs;

Reported by: Michael Heerdegen <michael_heerdegen <at> web.de>

Date: Sun, 30 Jun 2019 18:24:01 UTC

Severity: normal

Tags: fixed

Merged with 36321

Found in version 27.0.50

Done: Noam Postavsky <npostavs <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


Message #155 received at 36447 <at> debbugs.gnu.org (full text, mbox):

From: "Daniel Colascione" <dancol <at> dancol.org>
To: "Pip Cet" <pipcet <at> gmail.com>
Cc: michael_heerdegen <at> web.de, npostavs <at> gmail.com,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>,
 Daniel Colascione <dancol <at> dancol.org>, 36447 <at> debbugs.gnu.org
Subject: Re: bug#36447: 27.0.50; New "Unknown keyword" errors
Date: Wed, 10 Jul 2019 10:16:41 -0700
> On Wed, Jul 10, 2019 at 3:19 AM Daniel Colascione <dancol <at> dancol.org>
> wrote:
>> #1 works, but it's somewhat inefficient.
>>
>> #2 is a variant of #1, in a sense. Instead of copying the hash table
>> vectors and mutating them, we rebuild them from scratch. I don't
>> understand why we have to do that eagerly.
>
> I'm sorry if I suggested that we must do that. On the contrary, both
> options are entirely viable, though on balance I prefer the eager
> version.
>
> The lazy approach makes the code more complicated,

I don't think doing it eagerly is a complexity win. The eager patch posted
upthread is a wash.

> slightly slower,

No it doesn't. The hash table branch on negative count should be predicted
correctly and involves a test against a cache line we're loading anyway.
We can add explicit likely() and unlikely() annotations too.

> and introduced what appears to me to be at least one bug
> (Fhash_table_count returns a negative integer if called with a
> non-rehashed hastable).

That's a trivial oversight and not an inherent difficulty in the lazy
approach.

> The eager approach slows down startup by a fraction of a millisecond
> (it might be an entire millisecond if your Emacs session doesn't
> access any of the dumped hash tables at all, but since it does tend to
> do that, it'll be less in practice).

A millisecond here and a millisecond there and things end up costing real
time. A lazy approach isn't any harder than an eager one and

>> #1 isn't as bad as you might think.
>
> But it's not as good as "do #1 but only if PURE_P", which no longer works.
>
>> It's the same work either way, except that if we copy, when we grow the
>> hash table, we can actually free the original vectors.
>
> I'd like to restrict #1 to hash tables which are somehow immutable,
> either because they're pure or because we actually introduce immutable
> objects, so they'd never grow. Mutable hash tables must not share
> their index vectors anyway.

Did you miss the part about avoiding copy-on-write faults? Those hash
table vectors are going to be copied anyway, and we might as well do it
ourselves instead of making the kernel do it. (unexec has the same
inefficiency, BTW.) We should just do #1 for all hash tables.

>> IMHO, the right approach is to check in #1 for now and switch to a
>> #2-like
>> approach once master is stable. Noticing that we don't actually have to
>> store the hash table internal arrays in the dump is a good catch.
>
> I agree, but I think we should discuss the future of pure space, too.
> Maybe we should have a separate bug for that, though.

Sure.





This bug report was last modified 5 years and 316 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.