From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Eli Zaretskii <eliz@gnu.org>
Cc: michael_heerdegen@web.de, npostavs@gmail.com,
Pip Cet <pipcet@gmail.com>,
36447@debbugs.gnu.org
Subject: bug#36447: 27.0.50; New "Unknown keyword" errors
Date: Fri, 05 Jul 2019 14:00:22 -0400 [thread overview]
Message-ID: <jwvlfxcid18.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <83pnmoacft.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 05 Jul 2019 15:33:10 +0300")
> A naïve question: wouldn't the problem go away if we modified purecopy
> not to do the above, i.e. not to merge the next vector of a hash table
> with that of another?
It might do the trick, yes (the patch below does that, for example, tho
I think Pip's patch is a better approach).
Note that it would mean we still modify data in the purespace, which
ideally shouldn't happen.
The problem fundamentally is that `purecopy` assumes that the argument
passed to it will never again be modified (it doesn't have to be
immutable before, but it should be afterwards) but if we rehash
afterwards then we break this promise.
>> The (disappointingly trivial) fix:
>> call copy-sequence on h->next before rehashing the table.
> Rehashing is not only done during dumping, right?
The problem is not rehashing in general, but rehashing a purecopied
hash-table. This should normally never be needed, since a purecopied
hash-table should never be modified (no puthash/remhash should be
applied to it), but sadly pdumper may need to recompute the hashes
because objects's addresses may have changed between the dump and
the reload.
> So the fix you propose also makes rehashing slightly less efficient.
In my patch I added a comment to explain that hash_table_rehash is not
used in "normal" rehashing, but only in the rare case of rehashing on
the first access to a preloaded hash-table, so the efficiency
His patch looks good (probably better than mine).
His patch can/should be made slightly more efficient by only doing the
Fcopy_sequence on those hash-tables that are in purespace.
Stefan
diff --git a/src/fns.c b/src/fns.c
index 2fc000a7f4..cc4fd7f2d7 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4218,6 +4218,11 @@ maybe_resize_hash_table (struct Lisp_Hash_Table *h)
}
}
+/* Recompute the hashes (and hence also the "next" pointers).
+ Normally there's never a need to recompute hashes
+ This is done only on first-access to a hash-table loaded from
+ the "pdump", because the object's addresses may have changed, thus
+ affecting their hash. */
void
hash_table_rehash (struct Lisp_Hash_Table *h)
{
diff --git a/src/alloc.c b/src/alloc.c
index 64aaa8acdf..b0114a9459 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -5348,9 +5348,24 @@ purecopy_hash_table (struct Lisp_Hash_Table *table)
pure->header = table->header;
pure->weak = purecopy (Qnil);
+ /* After reloading the pdumped data, objects may ehave changed location
+ in memory, so their hash may have changed. So the `hash`, `next`, and
+ `index` vectors are not immutable and can't safely be hash-cons'ed. */
+ if (HASH_TABLE_P (Vpurify_flag))
+ {
+ Fremhash (table->hash, Vpurify_flag);
+ Fremhash (table->next, Vpurify_flag);
+ Fremhash (table->index, Vpurify_flag);
+ }
pure->hash = purecopy (table->hash);
pure->next = purecopy (table->next);
pure->index = purecopy (table->index);
+ if (HASH_TABLE_P (Vpurify_flag))
+ {
+ Fremhash (table->hash, Vpurify_flag);
+ Fremhash (table->next, Vpurify_flag);
+ Fremhash (table->index, Vpurify_flag);
+ }
pure->count = table->count;
pure->next_free = table->next_free;
pure->pure = table->pure;
next prev parent reply other threads:[~2019-07-05 18:00 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-30 18:23 bug#36447: 27.0.50; New "Unknown keyword" errors Michael Heerdegen
2019-06-30 18:43 ` Eli Zaretskii
2019-06-30 21:44 ` Michael Heerdegen
2019-07-01 12:25 ` Noam Postavsky
2019-07-01 13:20 ` Pip Cet
2019-07-01 22:04 ` Michael Heerdegen
2019-07-02 1:59 ` Stefan Kangas
2019-07-02 14:17 ` Eli Zaretskii
2019-07-02 13:29 ` Pip Cet
2019-07-02 15:35 ` Michael Heerdegen
2019-07-02 16:20 ` Noam Postavsky
2019-07-02 22:50 ` Pip Cet
2019-07-03 11:57 ` Pip Cet
2019-07-05 1:59 ` Michael Heerdegen
2019-07-05 6:35 ` Pip Cet
2019-07-05 7:50 ` Eli Zaretskii
2019-07-05 8:12 ` Pip Cet
2019-07-05 8:25 ` Eli Zaretskii
2019-07-05 8:36 ` Pip Cet
2019-07-05 8:41 ` Eli Zaretskii
2019-07-05 9:09 ` Pip Cet
2019-07-05 12:23 ` Robert Pluim
2019-07-05 12:33 ` Eli Zaretskii
2019-07-05 13:41 ` Pip Cet
2019-07-05 18:00 ` Stefan Monnier [this message]
2019-07-05 18:07 ` Eli Zaretskii
2019-07-05 20:16 ` Stefan Monnier
2019-07-05 18:57 ` Pip Cet
2019-07-05 19:13 ` Eli Zaretskii
2019-07-05 20:21 ` Stefan Monnier
2019-07-05 21:52 ` Pip Cet
2019-07-05 22:10 ` Stefan Monnier
2019-07-06 6:45 ` Eli Zaretskii
2019-07-06 15:08 ` Pip Cet
2019-07-09 21:05 ` Stefan Monnier
2019-07-10 2:38 ` Eli Zaretskii
2019-07-10 3:19 ` Daniel Colascione
2019-07-10 15:01 ` Pip Cet
2019-07-10 17:16 ` Daniel Colascione
2019-07-10 20:14 ` Pip Cet
2019-07-06 15:32 ` Michael Heerdegen
2019-07-08 17:30 ` Lars Ingebrigtsen
2019-07-08 17:58 ` Pip Cet
2019-07-08 22:18 ` Lars Ingebrigtsen
2019-07-08 22:25 ` Noam Postavsky
2019-07-09 14:00 ` Pip Cet
2019-07-10 3:01 ` Daniel Colascione
2019-07-14 14:06 ` Noam Postavsky
2019-07-08 23:22 ` Stefan Monnier
2019-07-08 22:23 ` Michael Heerdegen
2019-07-09 15:43 ` Eli Zaretskii
2019-07-09 20:15 ` Stefan Monnier
2019-07-05 7:55 ` Katsumi Yamaoka
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
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvlfxcid18.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=36447@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=michael_heerdegen@web.de \
--cc=npostavs@gmail.com \
--cc=pipcet@gmail.com \
/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 public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).