wingo pushed a commit to branch wip-whippet
in repository guile.
commit e51cf4bf654889cb06f579795ea98d3f092d684c
Author: Andy Wingo <wi...@pobox.com>
AuthorDate: Wed Jul 2 15:11:03 2025 +0200
Precise tracing works with heap-conservative-mmc
* libguile/ports.c (scm_make_port_type): Pin the ptob.
* libguile/threads.c (scm_trace_thread_roots): No need to trace dynstack
roots; we will do so within scm_trace_thread.
* libguile/trace.h (scm_trace_struct): Fix arg order to logbit?.
(scm_trace_ephemeron_table): Indentation fix.
(scm_trace_pointer): Actually trace the protected objects.
(scm_trace_vm):
(scm_trace_active_dynamic_state):
(scm_trace_thread): Fix to trace missing references.
(scm_trace_port): Trace the ptob and write_buf_aux.
(scm_trace_random_state): Add comment about rng being static.
* libguile/whippet-embedder.h (gc_is_valid_conservative_ref_displacement):
Don't hide behind an ifdef.
---
libguile/dynstack.c | 28 ---------------------
libguile/ports.c | 10 +++++---
libguile/threads.c | 3 ---
libguile/trace.h | 61 +++++++++++++++++++++++++++++----------------
libguile/whippet-embedder.h | 15 +++++------
5 files changed, 54 insertions(+), 63 deletions(-)
diff --git a/libguile/dynstack.c b/libguile/dynstack.c
index 7f9238f51..6c557c6a0 100644
--- a/libguile/dynstack.c
+++ b/libguile/dynstack.c
@@ -193,34 +193,6 @@ scm_trace_dynstack (struct scm_dynstack *dynstack,
}
}
-struct trace_pinned_trampoline
-{
- void (*trace_pinned) (struct gc_ref ref,
- struct gc_heap *heap,
- void *trace_data);
- void *trace_data;
-};
-
-static void
-trace_pinned_trampoline (struct gc_edge edge,
- struct gc_heap *heap,
- void *trace_data)
-{
- struct trace_pinned_trampoline *data = trace_data;
- return data->trace_pinned (gc_edge_ref (edge), heap, data->trace_data);
-}
-
-void
-scm_trace_dynstack_roots (struct scm_dynstack *dynstack,
- void (*trace_pinned) (struct gc_ref ref,
- struct gc_heap *heap,
- void *trace_data),
- struct gc_heap *heap, void *trace_data)
-{
- struct trace_pinned_trampoline data = { trace_pinned, trace_data };
- return scm_trace_dynstack (dynstack, trace_pinned_trampoline, heap, &data);
-}
-
static inline scm_t_bits *
push_dynstack_entry_unchecked (scm_t_dynstack *dynstack,
scm_t_dynstack_item_type type,
diff --git a/libguile/ports.c b/libguile/ports.c
index 5058e73c3..e83c5601b 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -248,10 +248,8 @@ scm_make_port_type (char *name,
size_t (*write) (SCM port, SCM src, size_t start,
size_t count))
{
- scm_t_port_type *desc;
-
- desc = scm_allocate_tagged (SCM_I_CURRENT_THREAD, sizeof (*desc));
- memset (desc, 0, sizeof (*desc));
+ struct scm_thread *thr = SCM_I_CURRENT_THREAD;
+ scm_t_port_type *desc = scm_allocate_tagged (thr, sizeof (*desc));
desc->tag = scm_tc7_port_type;
desc->name = name;
@@ -266,6 +264,10 @@ scm_make_port_type (char *name,
desc->stream_mode = SCM_PORT_STREAM_CONSERVATIVE;
scm_make_port_classes (desc);
+ // Since we have to reach into the ptob to get stream mode when
+ // collecting ports, let's pin the ptob.
+ scm_gc_pin_object (thr, SCM_PACK_POINTER (desc));
+
return desc;
}
diff --git a/libguile/threads.c b/libguile/threads.c
index 45e016625..85ccc7a57 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -107,9 +107,6 @@ scm_trace_thread_roots (struct scm_thread *thread,
struct gc_heap *heap, void *trace_data)
{
trace_pinned (gc_ref_from_heap_object (thread), heap, trace_data);
-#if GC_CONSERVATIVE_TRACE
- scm_trace_dynstack_roots (&thread->dynstack, trace_pinned, heap, trace_data);
-#endif
/* FIXME: Trace is not a tagged allocation. */
scm_trace_vm_roots (&thread->vm, trace_pinned, trace_ambiguous, heap,
trace_data);
diff --git a/libguile/trace.h b/libguile/trace.h
index 99cefbd76..b80bf09bc 100644
--- a/libguile/trace.h
+++ b/libguile/trace.h
@@ -116,14 +116,6 @@ scm_trace_loader_roots (void (*trace_ambiguous) (uintptr_t
lo,
struct gc_heap *heap,
void *trace_data);
-SCM_INTERNAL void
-scm_trace_dynstack_roots (struct scm_dynstack *dynstack,
- void (*trace_pinned) (struct gc_ref ref,
- struct gc_heap *heap,
- void *trace_data),
- struct gc_heap *heap,
- void *trace_data);
-
#define TRACE_PARAMS \
void (*trace) (struct gc_edge, struct gc_heap *, void *), \
struct gc_heap *heap, \
@@ -204,6 +196,8 @@ scm_trace_struct (struct scm_struct *s, TRACE_PARAMS)
if (trace)
{
+ // Because it's pinned, the source of the vtable edge doesn't matter.
+ TRACE (&vtable);
// unboxed_fields is either an inum or a bignum. If it's a
// bignum, it's pinned.
SCM unboxed_fields = scm_i_vtable_unboxed_fields (vtable);
@@ -212,7 +206,7 @@ scm_trace_struct (struct scm_struct *s, TRACE_PARAMS)
TRACE_SLOT (s->slots, i);
else
for (size_t i = 0; i < nfields; i++)
- if (scm_is_false (scm_logbit_p (unboxed_fields, SCM_I_MAKINUM (i))))
+ if (scm_is_false (scm_logbit_p (SCM_I_MAKINUM (i), unboxed_fields)))
TRACE_SLOT (s->slots, i);
}
@@ -253,10 +247,8 @@ static inline size_t
scm_trace_ephemeron_table (struct scm_ephemeron_table *et, TRACE_PARAMS)
{
if (trace)
- {
- for (size_t idx = 0; idx < et->size; idx++)
- TRACE_SLOT (et->contents, idx);
- }
+ for (size_t idx = 0; idx < et->size; idx++)
+ TRACE_SLOT (et->contents, idx);
return SIZEOF_WITH_TAIL (et, contents, et->size);
}
@@ -312,7 +304,7 @@ scm_trace_pointer (struct scm_pointer *p, TRACE_PARAMS)
if (trace)
for (size_t i = 0; i < extra_words; i++)
- TRACE_MEMBER (p, gc_objects);
+ TRACE_SLOT (p->gc_objects, i);
return SIZEOF_WITH_TAIL (p, gc_objects, extra_words);
}
@@ -451,18 +443,41 @@ scm_trace_bytevector (struct scm_bytevector *x,
TRACE_PARAMS)
return sizeof (*x);
}
+static inline void
+scm_trace_vm (struct scm_vm *x, TRACE_PARAMS)
+{
+ TRACE_MEMBER (x, apply_hook);
+ TRACE_MEMBER (x, return_hook);
+ TRACE_MEMBER (x, next_hook);
+ TRACE_MEMBER (x, abort_hook);
+ TRACE_MEMBER (x, overflow_handler_stack);
+}
+
+static inline void
+scm_trace_active_dynamic_state (struct scm_dynamic_state *x, TRACE_PARAMS)
+{
+ TRACE_MEMBER (x, thread_local_values);
+ TRACE_MEMBER (x, values);
+ for (size_t i = 0; i < SCM_CACHE_SIZE; i++)
+ {
+ TRACE (&x->cache.entries[i].key);
+ TRACE (&x->cache.entries[i].value);
+ }
+}
+
static inline size_t
scm_trace_thread (struct scm_thread *x, TRACE_PARAMS)
{
if (trace)
{
TRACE_MEMBER (x, next_thread);
+ scm_trace_vm (&x->vm, TRACE_ARGS);
TRACE_MEMBER (x, pending_asyncs);
+ scm_trace_active_dynamic_state (&x->dynamic_state, TRACE_ARGS);
TRACE_MEMBER (x, continuation_root);
TRACE_MEMBER (x, join_cond);
TRACE_MEMBER (x, join_lock);
TRACE_MEMBER (x, join_results);
-
scm_trace_dynstack (&x->dynstack, TRACE_ARGS);
}
@@ -525,18 +540,22 @@ scm_trace_port (struct scm_t_port *x, TRACE_PARAMS)
{
if (trace)
{
+ TRACE_MEMBER (x, ptob);
+
+ // ptob is pinned.
+ if (x->ptob->stream_mode != SCM_PORT_STREAM_UNMANAGED)
+ TRACE_MEMBER (x, stream);
+
TRACE_MEMBER (x, file_name);
TRACE_MEMBER (x, position);
TRACE_MEMBER (x, read_buf);
TRACE_MEMBER (x, write_buf);
+ TRACE_MEMBER (x, write_buf_aux);
TRACE_MEMBER (x, encoding);
TRACE_MEMBER (x, conversion_strategy);
TRACE_MEMBER (x, precise_encoding);
TRACE_MEMBER (x, close_handle);
TRACE_MEMBER (x, alist);
-
- if (x->ptob->stream_mode != SCM_PORT_STREAM_UNMANAGED)
- TRACE_MEMBER (x, stream);
}
return sizeof (*x);
@@ -577,11 +596,10 @@ scm_trace_continuation (struct scm_continuation *c,
TRACE_PARAMS)
{
if (trace)
{
- TRACE_MEMBER (c, root);
- TRACE_MEMBER (c, vm_cont);
-
scm_trace_range_conservatively (&c->jmpbuf, sizeof (c->jmpbuf), 1,
TRACE_ARGS);
+ TRACE_MEMBER (c, root);
+ TRACE_MEMBER (c, vm_cont);
scm_trace_range_conservatively (c->stack,
c->num_stack_items * sizeof
(SCM_STACKITEM),
1,
@@ -613,6 +631,7 @@ scm_trace_syntax_transformer (struct scm_syntax_transformer
*tx, TRACE_PARAMS)
static inline size_t
scm_trace_random_state (struct scm_t_rstate *s, TRACE_PARAMS)
{
+ // rng is off-heap.
return s->rng->rstate_size;
}
diff --git a/libguile/whippet-embedder.h b/libguile/whippet-embedder.h
index b7e733416..bd6c9f934 100644
--- a/libguile/whippet-embedder.h
+++ b/libguile/whippet-embedder.h
@@ -49,15 +49,16 @@ static inline size_t gc_finalizer_priority_count (void) {
return 2; }
static inline int
gc_is_valid_conservative_ref_displacement (uintptr_t displacement) {
-#if GC_CONSERVATIVE_ROOTS || GC_CONSERVATIVE_TRACE
- if (displacement == 0) return 1;
- if (displacement == scm_tc3_cons) return 1;
- if (displacement == scm_tc3_struct) return 1;
- return 0;
-#else
+ if (GC_CONSERVATIVE_ROOTS || GC_CONSERVATIVE_TRACE)
+ {
+ if (displacement == 0) return 1;
+ if (displacement == scm_tc3_cons) return 1;
+ if (displacement == scm_tc3_struct) return 1;
+ return 0;
+ }
+
// Shouldn't get here.
GC_CRASH ();
-#endif
}
// FIXME: Here add tracing for SCM literals in .go files or .data
