wingo pushed a commit to branch wip-whippet
in repository guile.
commit c9063b8027ff9b7f3bc033f61320905933f0a7c8
Author: Andy Wingo <wi...@igalia.com>
AuthorDate: Wed Apr 23 17:28:46 2025 +0200
bdw: Disable thread-local freelists for pointerless allocations
There was a race between GC and popping the head off freelists. We
could have separate freelists for tagged and untagged normal allocations
though.
---
api/bdw-attrs.h | 18 +++++++++-------
api/gc-api.h | 8 +++----
api/gc-attrs.h | 10 ++++-----
api/mmc-attrs.h | 4 ++--
api/pcc-attrs.h | 4 ++--
api/semi-attrs.h | 5 +++--
src/bdw.c | 64 ++++++++++++++++++++------------------------------------
7 files changed, 50 insertions(+), 63 deletions(-)
diff --git a/api/bdw-attrs.h b/api/bdw-attrs.h
index 7f8000b3f..7a68618cc 100644
--- a/api/bdw-attrs.h
+++ b/api/bdw-attrs.h
@@ -4,8 +4,15 @@
#include "gc-attrs.h"
#include "gc-assert.h"
-static inline enum gc_allocator_kind gc_allocator_kind(void) {
- return GC_ALLOCATOR_INLINE_FREELIST;
+static inline enum gc_inline_allocator_kind
+gc_inline_allocator_kind(enum gc_allocation_kind kind) {
+ switch (kind) {
+ case GC_ALLOCATION_TAGGED:
+ case GC_ALLOCATION_UNTAGGED_CONSERVATIVE:
+ return GC_INLINE_ALLOCATOR_FREELIST;
+ default:
+ return GC_INLINE_ALLOCATOR_NONE;
+ }
}
static inline size_t gc_allocator_small_granule_size(void) {
return 2 * sizeof(void *);
@@ -30,11 +37,8 @@ static inline size_t gc_allocator_freelist_offset(size_t
size,
case GC_ALLOCATION_UNTAGGED_CONSERVATIVE:
base = 0;
break;
- case GC_ALLOCATION_UNTAGGED_POINTERLESS:
- case GC_ALLOCATION_TAGGED_POINTERLESS:
- base = (sizeof(void*) * gc_allocator_large_threshold() /
- gc_allocator_small_granule_size());
- break;
+ default:
+ GC_CRASH();
}
size_t bucket = (size - 1) / gc_allocator_small_granule_size();
return base + sizeof(void*) * bucket;
diff --git a/api/gc-api.h b/api/gc-api.h
index 58f40b779..b040b7d0a 100644
--- a/api/gc-api.h
+++ b/api/gc-api.h
@@ -154,12 +154,12 @@ static inline void* gc_allocate_small_fast(struct
gc_mutator *mut, size_t size,
GC_ASSERT(size != 0);
GC_ASSERT(size <= gc_allocator_large_threshold());
- switch (gc_allocator_kind()) {
- case GC_ALLOCATOR_INLINE_BUMP_POINTER:
+ switch (gc_inline_allocator_kind(kind)) {
+ case GC_INLINE_ALLOCATOR_BUMP_POINTER:
return gc_allocate_small_fast_bump_pointer(mut, size, kind);
- case GC_ALLOCATOR_INLINE_FREELIST:
+ case GC_INLINE_ALLOCATOR_FREELIST:
return gc_allocate_small_fast_freelist(mut, size, kind);
- case GC_ALLOCATOR_INLINE_NONE:
+ case GC_INLINE_ALLOCATOR_NONE:
return NULL;
default:
GC_CRASH();
diff --git a/api/gc-attrs.h b/api/gc-attrs.h
index 4c801c1c8..1ce2c854b 100644
--- a/api/gc-attrs.h
+++ b/api/gc-attrs.h
@@ -7,13 +7,13 @@
#include <stddef.h>
#include <stdint.h>
-enum gc_allocator_kind {
- GC_ALLOCATOR_INLINE_BUMP_POINTER,
- GC_ALLOCATOR_INLINE_FREELIST,
- GC_ALLOCATOR_INLINE_NONE
+enum gc_inline_allocator_kind {
+ GC_INLINE_ALLOCATOR_BUMP_POINTER,
+ GC_INLINE_ALLOCATOR_FREELIST,
+ GC_INLINE_ALLOCATOR_NONE
};
-static inline enum gc_allocator_kind gc_allocator_kind(void) GC_ALWAYS_INLINE;
+static inline enum gc_inline_allocator_kind gc_inline_allocator_kind(enum
gc_allocation_kind) GC_ALWAYS_INLINE;
static inline size_t gc_allocator_large_threshold(void) GC_ALWAYS_INLINE;
static inline size_t gc_allocator_small_granule_size(void) GC_ALWAYS_INLINE;
diff --git a/api/mmc-attrs.h b/api/mmc-attrs.h
index 9371f8abe..49ff5630b 100644
--- a/api/mmc-attrs.h
+++ b/api/mmc-attrs.h
@@ -5,8 +5,8 @@
#include "gc-assert.h"
#include "gc-attrs.h"
-static inline enum gc_allocator_kind gc_allocator_kind(void) {
- return GC_ALLOCATOR_INLINE_BUMP_POINTER;
+static inline enum gc_inline_allocator_kind gc_inline_allocator_kind(enum
gc_allocation_kind kind) {
+ return GC_INLINE_ALLOCATOR_BUMP_POINTER;
}
static inline size_t gc_allocator_small_granule_size(void) {
return 16;
diff --git a/api/pcc-attrs.h b/api/pcc-attrs.h
index 12a555a5d..d7ec30768 100644
--- a/api/pcc-attrs.h
+++ b/api/pcc-attrs.h
@@ -8,8 +8,8 @@
static const uintptr_t GC_ALIGNMENT = 8;
static const size_t GC_LARGE_OBJECT_THRESHOLD = 8192;
-static inline enum gc_allocator_kind gc_allocator_kind(void) {
- return GC_ALLOCATOR_INLINE_BUMP_POINTER;
+static inline enum gc_inline_allocator_kind gc_inline_allocator_kind(enum
gc_allocation_kind kind) {
+ return GC_INLINE_ALLOCATOR_BUMP_POINTER;
}
static inline size_t gc_allocator_small_granule_size(void) {
return GC_ALIGNMENT;
diff --git a/api/semi-attrs.h b/api/semi-attrs.h
index f2efbd831..fb4d7ab91 100644
--- a/api/semi-attrs.h
+++ b/api/semi-attrs.h
@@ -7,8 +7,9 @@
static const uintptr_t GC_ALIGNMENT = 8;
static const size_t GC_LARGE_OBJECT_THRESHOLD = 8192;
-static inline enum gc_allocator_kind gc_allocator_kind(void) {
- return GC_ALLOCATOR_INLINE_BUMP_POINTER;
+static inline enum gc_inline_allocator_kind
+gc_inline_allocator_kind(enum gc_allocation_kind kind) {
+ return GC_INLINE_ALLOCATOR_BUMP_POINTER;
}
static inline size_t gc_allocator_small_granule_size(void) {
return GC_ALIGNMENT;
diff --git a/src/bdw.c b/src/bdw.c
index 886e70718..f991437fd 100644
--- a/src/bdw.c
+++ b/src/bdw.c
@@ -65,7 +65,6 @@ struct gc_heap {
struct gc_mutator {
void *freelists[GC_INLINE_FREELIST_COUNT];
- void *pointerless_freelists[GC_INLINE_FREELIST_COUNT];
struct gc_heap *heap;
struct gc_mutator_roots *roots;
struct gc_mutator *next; // with heap lock
@@ -124,9 +123,6 @@ allocate_small(void **freelist, size_t idx, enum
gc_inline_kind kind) {
*freelist = *(void **)(head);
*(void**)head = NULL;
- if (kind == GC_INLINE_KIND_POINTERLESS)
- memset(head, 0, gc_inline_freelist_object_size(idx));
-
return head;
}
@@ -134,41 +130,38 @@ void* gc_allocate_slow(struct gc_mutator *mut, size_t
size,
enum gc_allocation_kind kind) {
GC_ASSERT(size != 0);
if (size <= gc_allocator_large_threshold()) {
- size_t idx = gc_inline_bytes_to_freelist_index(size);
- void **freelists;
- enum gc_inline_kind freelist_kind;
switch (kind) {
case GC_ALLOCATION_TAGGED:
- case GC_ALLOCATION_UNTAGGED_CONSERVATIVE:
+ case GC_ALLOCATION_UNTAGGED_CONSERVATIVE: {
+ size_t idx = gc_inline_bytes_to_freelist_index(size);
return allocate_small(&mut->freelists[idx], idx,
GC_INLINE_KIND_NORMAL);
+ }
case GC_ALLOCATION_TAGGED_POINTERLESS:
case GC_ALLOCATION_UNTAGGED_POINTERLESS:
- return allocate_small(&mut->pointerless_freelists[idx], idx,
- GC_INLINE_KIND_POINTERLESS);
+ break;
default:
GC_CRASH();
}
- } else {
- switch (kind) {
- case GC_ALLOCATION_TAGGED:
- case GC_ALLOCATION_UNTAGGED_CONSERVATIVE: {
- void *ret = GC_malloc(size);
- if (GC_LIKELY (ret != NULL))
- return ret;
- return __the_bdw_gc_heap->allocation_failure(__the_bdw_gc_heap, size);
- }
- case GC_ALLOCATION_TAGGED_POINTERLESS:
- case GC_ALLOCATION_UNTAGGED_POINTERLESS: {
- void *ret = GC_malloc_atomic(size);
- if (GC_LIKELY (ret != NULL)) {
- memset(ret, 0, size);
- return ret;
- }
- return __the_bdw_gc_heap->allocation_failure(__the_bdw_gc_heap, size);
+ }
+ switch (kind) {
+ case GC_ALLOCATION_TAGGED:
+ case GC_ALLOCATION_UNTAGGED_CONSERVATIVE: {
+ void *ret = GC_malloc(size);
+ if (GC_LIKELY (ret != NULL))
+ return ret;
+ return __the_bdw_gc_heap->allocation_failure(__the_bdw_gc_heap, size);
+ }
+ case GC_ALLOCATION_TAGGED_POINTERLESS:
+ case GC_ALLOCATION_UNTAGGED_POINTERLESS: {
+ void *ret = GC_malloc_atomic(size);
+ if (GC_LIKELY (ret != NULL)) {
+ memset(ret, 0, size);
+ return ret;
}
- default:
- GC_CRASH();
+ return __the_bdw_gc_heap->allocation_failure(__the_bdw_gc_heap, size);
}
+ default:
+ GC_CRASH();
}
}
@@ -400,18 +393,7 @@ mark_mutator(GC_word *addr, struct GC_ms_entry
*mark_stack_ptr,
return state.mark_stack_ptr;
}
- for (int i = 0; i < GC_INLINE_FREELIST_COUNT; i++)
- state.mark_stack_ptr = GC_MARK_AND_PUSH (mut->freelists[i],
- state.mark_stack_ptr,
- state.mark_stack_limit,
- NULL);
-
- for (int i = 0; i < GC_INLINE_FREELIST_COUNT; i++)
- for (void *head = mut->pointerless_freelists[i]; head; head =
*(void**)head)
- state.mark_stack_ptr = GC_MARK_AND_PUSH (head,
- state.mark_stack_ptr,
- state.mark_stack_limit,
- NULL);
+ memset(mut->freelists, 0, sizeof(void*) * GC_INLINE_FREELIST_COUNT);
if (mut->roots)
gc_trace_mutator_roots(mut->roots, bdw_mark_edge, mut->heap, &state);
