From: ludo@gnu.org (Ludovic Courtès)
To: Andy Wingo <wingo@igalia.com>
Cc: 19180@debbugs.gnu.org,
Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>,
guile-devel@gnu.org
Subject: bug#19180: Weak tables harmful to GC?
Date: Tue, 31 Oct 2017 00:13:51 +0100 [thread overview]
Message-ID: <87zi88ut3k.fsf__42200.0722297982$1509405258$gmane$org@gnu.org> (raw)
In-Reply-To: <87she0lf1y.fsf@igalia.com> (Andy Wingo's message of "Mon, 30 Oct 2017 18:29:45 +0100")
Hi Andy,
Andy Wingo <wingo@igalia.com> skribis:
> As discussed on IRC, what do you think of this patch? It preserves the
> thread-safety properties of weak tables and just adapts them to be
> bucket-and-chain tables. Let me know how it works for you.
That was fast! The code certainly looks nicer than the old entangled
weak hash table code, and it preserves thread-safety, so that’s great.
> If it works, we'll need to adapt weak sets as well.
Yes, though I think weak sets are less critical.
I built libguile with the patch (I haven’t yet taken the time to rebuild
all of Guile). It works, but unfortunately it still shows quick growth
of the heap on this example (<https://bugs.gnu.org/28590>):
--8<---------------cut here---------------start------------->8---
(use-modules (ice-9 format))
(define (display-heap-size)
(format #t "heap size: ~,2h MiB~%"
(/ (assoc-ref (gc-stats) 'heap-size) (expt 2. 20))))
(define table
(make-weak-key-hash-table))
(let loop ((i 0))
(unless #f
(when (zero? (modulo i 100000))
(pk 'table table)
(display-heap-size))
(hashq-set! table (make-list 10) (make-list 10))
(loop (1+ i))))
--8<---------------cut here---------------end--------------->8---
Could it me that some of the disappearing links are not getting
unregistered?
> From 6ec4642516eaabf7a63644463a7836eb3efbcd60 Mon Sep 17 00:00:00 2001
> From: Andy Wingo <wingo@pobox.com>
> Date: Mon, 30 Oct 2017 18:19:37 +0100
> Subject: [PATCH] Weak tables are now bucket-and-chain tables
>
> This change should make weak tables work better with libgc, as the weak
> components that need mark functions are smaller, so they don't overflow
> the mark queue. Also this prevents the need to move disappearing
> links.
>
> * libguile/weak-table.c (scm_t_weak_entry): Change to be a hash table
> chain entry.
> (struct weak_entry_data, do_read_weak_entry, read_weak_entry): Read
> out the key and value directly.
> (GC_move_disappearing_link, move_disappearing_links, move_weak_entry):
> Remove.
> (scm_t_weak_table): Rename "entries" member to "buckets", and "size" to
> "n_buckets".
> (hash_to_index, entry_distance, rob_from_rich, give_to_poor): Remove.
> (mark_weak_key_entry, mark_weak_value_entry): Mark a single link, and
> the next link.
> (mark_doubly_weak_entry): New kind.
> (allocate_entry): Allocate a single entry.
> (add_entry): New helper.
> (resize_table): Reimplement more like normal hash tables.
> (vacuum_weak_table): Adapt to new implementation.
> (weak_table_ref, weak_table_put_x, weak_table_remove_x): Adapt.
> (make_weak_table): Adapt.
> (scm_weak_table_clear_x): Actually unregister the links to prevent a
> memory leak.
> (scm_c_weak_table_fold): Collect items in an alist, then fold outside
> the lock.
> (scm_weak_table_prehistory): Initialize doubly_weak_gc_kind.
[...]
> +mark_weak_key_entry (GC_word *addr, struct GC_ms_entry *mark_stack_ptr,
> struct GC_ms_entry *mark_stack_limit, GC_word env)
[...]
> weak_key_gc_kind =
> GC_new_kind (GC_new_free_list (),
> - GC_MAKE_PROC (GC_new_proc (mark_weak_key_table), 0),
> + GC_MAKE_PROC (GC_new_proc (mark_weak_key_entry), 0),
> 0, 0);
I think we should avoid custom mark procedures and use a bitmap here, as
recommended in the libgc headers (like ‘wcar_pair_descr’ in weaks.c in
2.0.)
Other than that, it looks good on a cursory look. We’ll have to do some
more testing afterwards to gain more confidence, like what Ricardo has
been doing.
Thanks a lot for your help!
Ludo’.
next prev parent reply other threads:[~2017-10-30 23:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <87wp9gwz8m.fsf@gnu.org>
[not found] ` <87fug4o8z2.fsf@pobox.com>
[not found] ` <87wp54z3p5.fsf@gnu.org>
[not found] ` <87zi9th1i6.fsf_-_@gnu.org>
[not found] ` <87y3o454pr.fsf@gnu.org>
[not found] ` <87r2tvncs5.fsf@dustycloud.org>
[not found] ` <87wp3mwwef.fsf@gnu.org>
[not found] ` <87po9c88rx.fsf@dustycloud.org>
[not found] ` <87o9owm9v1.fsf@gnu.org>
2017-10-25 0:50 ` bug#19180: Weak tables harmful to GC? Christopher Allan Webber
[not found] ` <87o9ow830m.fsf@dustycloud.org>
2017-10-25 17:11 ` Ludovic Courtès
[not found] ` <87mv4gd0ik.fsf@elephly.net>
2017-10-25 6:38 ` Ricardo Wurmus
2017-10-26 7:03 ` Ludovic Courtès
[not found] ` <87a80eie63.fsf@gnu.org>
2017-10-26 8:35 ` Ricardo Wurmus
[not found] ` <87k1zimhmt.fsf@elephly.net>
2017-10-26 16:52 ` Ricardo Wurmus
2017-10-26 17:17 ` Ludovic Courtès
[not found] ` <87bmktn96e.fsf@elephly.net>
2017-10-27 5:28 ` Ludovic Courtès
[not found] ` <87tvyl9n22.fsf@gnu.org>
2017-10-28 9:56 ` Ricardo Wurmus
[not found] ` <87r2tnlhno.fsf@elephly.net>
2017-10-30 12:35 ` Ludovic Courtès
[not found] ` <87a8087qz7.fsf@gnu.org>
2017-10-30 14:48 ` Ricardo Wurmus
[not found] ` <87d154lmio.fsf@mdc-berlin.de>
2017-10-30 17:20 ` Ricardo Wurmus
[not found] ` <878tfslfgl.fsf@mdc-berlin.de>
2017-10-30 22:18 ` Ludovic Courtès
2017-10-30 17:29 ` Andy Wingo
2017-10-30 23:13 ` Ludovic Courtès [this message]
[not found] ` <87zi88ut3k.fsf@gnu.org>
2017-10-31 8:25 ` Andy Wingo
[not found] ` <877evbiuzy.fsf@igalia.com>
2017-10-31 16:56 ` Ludovic Courtès
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/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='87zi88ut3k.fsf__42200.0722297982$1509405258$gmane$org@gnu.org' \
--to=ludo@gnu.org \
--cc=19180@debbugs.gnu.org \
--cc=guile-devel@gnu.org \
--cc=ricardo.wurmus@mdc-berlin.de \
--cc=wingo@igalia.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.
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).