Remove two arguments from gc_alloc_large

- It doesn't really use an alloc_region; it's confusing to supply one.
- It can figure out whether to use a mutex based on gc_active_p.
- Rearrange lisp_alloc so that if calling alloc_large it's a tail call.
- Fix busted GC tests (Why are they still not run with the test suite?)

As part of the change though only tangentially related: when sb-sprof
is run in allocation profiling mode, don't record a trace for PAGE_TYPE_CODE
because there's only one code region which messes up the per-thread stats.
This commit is contained in:
Douglas Katzman 2022-05-18 21:39:57 -04:00
parent cee3f22c13
commit 5d798d95f3
4 changed files with 83 additions and 71 deletions

View file

@ -107,7 +107,7 @@ heap-reloc-test: ../../tests/heap-reloc/fake-mman.c $(OBJS)
# Enable compiling gencgc with even more assertions and/or
# data collection, with COMPILING_TESTS. Not really used yet.
gc-unit-tests.o: CFLAGS=-DCOMPILING_TESTS
gc-unit-tests.o: CFLAGS=-g -DCOMPILING_TESTS
unit-tests: gc-unit-tests.o libsbcl.a
cc -g -no-pie -o $@ $^ -ldl -lpthread -lm

View file

@ -92,7 +92,6 @@ void test_adjust_obj_ptes()
// and pick the generation.
gc_init_region(&test_region);
RESET_ALLOC_START_PAGES();
test_region.last_page = -1;
gc_alloc_generation = SCRATCH_GENERATION;
// Wipe out the page table and the allocation counts,
@ -102,7 +101,7 @@ void test_adjust_obj_ptes()
for (gen=0; gen < NUM_GENERATIONS; ++gen)
generations[gen].bytes_allocated = 0;
bytes_allocated = 0;
void *result = gc_alloc_large(request, PAGE_TYPE_UNBOXED, &test_region, 0);
void *result = gc_alloc_large(request, PAGE_TYPE_UNBOXED);
// Assert some things about the reference object.
gc_assert(result == (void*)DYNAMIC_SPACE_START);
@ -141,12 +140,12 @@ void shrink_obj_test(int ending_size, int initial_type,
if (initial_size >= ending_size) {
gc_init_region(&test_region);
RESET_ALLOC_START_PAGES();
test_region.last_page = -1;
// Start with a fresh page table
page_table = calloc(1+page_table_pages, sizeof(struct page));
gc_page_pins = calloc(page_table_pages, 1);
from_space = gc_alloc_generation = 2;
void *result = gc_alloc_large(initial_size, initial_type, &test_region, 0);
void *result = gc_alloc_large(initial_size, initial_type);
// We're in trouble if pages other than expected were gotten
gc_assert(result == (void*)DYNAMIC_SPACE_START);

View file

@ -836,8 +836,18 @@ struct alloc_region gc_alloc_region[6];
static page_index_t
alloc_start_pages[8], // one for each value of PAGE_TYPE_x
gencgc_alloc_start_page; // initializer for the preceding array
gencgc_alloc_start_page, // initializer for the preceding array
max_alloc_start_page; // the largest of any array element
/* Each 'start_page' informs the region-opening logic where it should
* attempt to continue allocating after closing a region associated
* with a particular page type. We aren't very clever about this -
* either the start_page has space remaining or it doesn't, and when it
* doesn't, then we should hop over *all* allocated pages regardless of
* type that intercede between the page we couldn't use up to next_free_page.
* It's kind of dumb that there is one start_page per type,
* other than it serves its purpose for picking up where it left off
* on a partially full page during GC */
#define RESET_ALLOC_START_PAGES() \
alloc_start_pages[0] = gencgc_alloc_start_page; \
alloc_start_pages[1] = gencgc_alloc_start_page; \
@ -846,20 +856,22 @@ static page_index_t
alloc_start_pages[4] = gencgc_alloc_start_page; \
alloc_start_pages[5] = gencgc_alloc_start_page; \
alloc_start_pages[6] = gencgc_alloc_start_page; \
alloc_start_pages[7] = gencgc_alloc_start_page;
alloc_start_pages[7] = gencgc_alloc_start_page; \
max_alloc_start_page = gencgc_alloc_start_page;
static inline page_index_t
alloc_start_page(unsigned int page_type, int large)
get_alloc_start_page(unsigned int page_type)
{
if (page_type > 7) lose("bad page_type: %d", page_type);
return alloc_start_pages[large ? 0 : page_type];
return alloc_start_pages[page_type];
}
static inline void
set_alloc_start_page(unsigned int page_type, int large, page_index_t page)
set_alloc_start_page(unsigned int page_type, page_index_t page)
{
if (page_type > 7) lose("bad page_type: %d", page_type);
alloc_start_pages[large ? 0 : page_type] = page;
if (page > max_alloc_start_page) max_alloc_start_page = page;
alloc_start_pages[page_type] = page;
}
#include "private-cons.inc"
@ -1027,7 +1039,7 @@ gc_alloc_new_region(sword_t nbytes, int page_type, struct alloc_region *alloc_re
return alloc_region->free_pointer;
}
page_index_t first_page = alloc_start_page(page_type, 0), last_page;
page_index_t first_page = get_alloc_start_page(page_type), last_page;
INSTRUMENTING(
last_page = gc_find_freeish_pages(&first_page, nbytes,
@ -1242,7 +1254,7 @@ gc_close_region(struct alloc_region *alloc_region, int page_type)
generations[gc_alloc_generation].bytes_allocated += region_size;
/* Set the alloc restart page to the last page of the region. */
set_alloc_start_page(page_type, 0, next_page-1);
set_alloc_start_page(page_type, next_page-1);
/* Add the region to the new_areas if requested. */
if (boxed_type_p(page_type))
@ -1264,32 +1276,28 @@ gc_close_region(struct alloc_region *alloc_region, int page_type)
}
/* Allocate a possibly large object. */
void *
gc_alloc_large(sword_t nbytes, int page_type, struct alloc_region *alloc_region, int unlock)
void *gc_alloc_large(sword_t nbytes, int page_type)
{
page_index_t first_page, last_page;
// Large BOXED would serve no purpose beyond MIXED, and "small large" is illogical.
if (page_type == PAGE_TYPE_BOXED || page_type == PAGE_TYPE_SMALL_MIXED)
page_type = PAGE_TYPE_MIXED;
first_page = alloc_start_page(page_type, 1);
// FIXME: really we want to try looking for space following the highest of
// the last page of all other small object regions. That's impossible - there's
// not enough information. At best we can skip some work in only the case where
// the supplied region was the one most recently created. To do this right
// would entail a malloc-like allocator at the page granularity.
page_index_t min = find_page_index(alloc_region->end_addr);
if (first_page < min) first_page = min;
int locked = !gc_active_p;
if (locked) {
int __attribute__((unused)) ret = mutex_acquire(&free_pages_lock);
gc_assert(ret);
}
first_page = max_alloc_start_page;
INSTRUMENTING(
last_page = gc_find_freeish_pages(&first_page, nbytes,
SINGLE_OBJECT_FLAG | page_type,
gc_alloc_generation),
et_find_freeish_page);
// FIXME: Should this be 1+last_page ?
// (Doesn't matter too much since it'll be skipped on restart if unusable)
set_alloc_start_page(page_type, 1, last_page);
// No need to check whether last_page > old max; it's gotta be.
max_alloc_start_page = last_page;
/* Set up the pages. */
page_index_t page;
@ -1310,10 +1318,10 @@ gc_alloc_large(sword_t nbytes, int page_type, struct alloc_region *alloc_region,
// Anyway it's best if the new page resembles a valid object ASAP.
uword_t nwords = nbytes >> WORD_SHIFT;
lispobj* addr = (lispobj*)page_address(first_page);
if (unlock)
if (locked)
THREAD_JIT(0);
*addr = (nwords - 1) << N_WIDETAG_BITS | FILLER_WIDETAG;
if (unlock) // avoid enabling while GCing
if (locked) // avoid enabling while GCing
THREAD_JIT(1);
os_vm_size_t scan_start_offset = 0;
@ -1330,7 +1338,7 @@ gc_alloc_large(sword_t nbytes, int page_type, struct alloc_region *alloc_region,
bytes_allocated += nbytes;
generations[gc_alloc_generation].bytes_allocated += nbytes;
if (unlock) {
if (locked) {
int __attribute__((unused)) ret = mutex_release(&free_pages_lock);
gc_assert(ret);
}
@ -1525,7 +1533,7 @@ void *collector_alloc_fallback(struct alloc_region* region, sword_t nbytes, int
* because genesis does not use large-object pages. So cold-init could fail,
* depending on whether objects in the cold core are sufficiently large that
* they ought to have gone on large object pages if they could have. */
if (nbytes >= LARGE_OBJECT_SIZE) return gc_alloc_large(nbytes, page_type, region, 0);
if (nbytes >= LARGE_OBJECT_SIZE) return gc_alloc_large(nbytes, page_type);
if (page_type != PAGE_TYPE_SMALL_MIXED) return new_region(region, nbytes, page_type);
@ -4842,10 +4850,14 @@ lisp_alloc(int largep, struct alloc_region *region, sword_t nbytes,
region->free_pointer = new_free_pointer;
#if defined LISP_FEATURE_MIPS || defined LISP_FEATURE_PPC || \
defined LISP_FEATURE_PPC64 || defined LISP_FEATURE_X86_64
// Most allocations should never get here, but two page types are special.
/* Most allocations should never get here, but two page types are special.
* - CODE always comes through here.
* - CONS can come through here because when overflow occurs in lisp,
* the fallback logic will call lisp_alloc one or more times,
* obtaining possibly discontiguous pages of conses */
gc_assert(page_type == PAGE_TYPE_CONS || page_type == PAGE_TYPE_CODE);
#endif
return(new_obj); /* yup */
return new_obj;
}
/* We don't want to count nbytes against auto_gc_trigger unless we
@ -4887,49 +4899,51 @@ lisp_alloc(int largep, struct alloc_region *region, sword_t nbytes,
}
}
}
int __attribute__((unused)) ret = mutex_acquire(&free_pages_lock);
gc_assert(ret);
if (largep)
new_obj = gc_alloc_large(nbytes, page_type, region, 1);
else {
ensure_region_closed(region, page_type);
// hold the lock after alloc_new_region if a cons page
int release = page_type != PAGE_TYPE_CONS;
new_obj = gc_alloc_new_region(nbytes, page_type, region, release);
region->free_pointer = (char*)new_obj + nbytes;
// addr_diff asserts that 'end' >= 'free_pointer'
int remaining = addr_diff(region->end_addr, region->free_pointer);
// Try to avoid the next Lisp -> C -> Lisp round-trip by possibly
// requesting yet another region.
if (page_type == PAGE_TYPE_CONS) {
if (remaining <= CONS_SIZE * N_WORD_BYTES) { // Refill now if <= 1 more cons to go
gc_close_region(region, page_type);
// Request > 2 words, forcing a new page to be claimed.
gc_alloc_new_region(4 * N_WORD_BYTES, page_type, region, 0); // don't release
}
ret = mutex_release(&free_pages_lock);
gc_assert(ret);
} else if (remaining <= 4 * N_WORD_BYTES
&& TryEnterCriticalSection(&free_pages_lock)) {
gc_close_region(region, page_type);
// Request > 4 words, forcing a new page to be claimed.
gc_alloc_new_region(6 * N_WORD_BYTES, page_type, region, 1); // do release
}
}
/* For the architectures which do NOT use a trap instruction for allocation,
* overflow, record a backtrace now if statistical profiling is enabled.
* The ones which use a trap will backtrace from the signal handler.
* Code allocations are ignored, because every code allocation
* comes through lisp_alloc() which makes this not a statistical
* sample. Also the trapping ones don't trap for code.
* #+win32 doesn't seem to work, but neither does CPU profiling */
#if !(defined LISP_FEATURE_PPC || defined LISP_FEATURE_PPC64 \
|| defined LISP_FEATURE_SPARC || defined LISP_FEATURE_WIN32)
// Architectures which utilize a trap instruction to invoke the overflow
// handler use the signal context from which to record a backtrace.
// That's reliable, but access_control_frame_pointer(thread) isn't.
// x86[-64] use the ABI frame pointer register which seems not to work
// for win32, but sb-sprof never did work there anyway.
extern void allocator_record_backtrace(void*, struct thread*);
if (gencgc_alloc_profiler && thread->state_word.sprof_enable)
if (page_type != PAGE_TYPE_CODE && gencgc_alloc_profiler
&& thread->state_word.sprof_enable)
allocator_record_backtrace(__builtin_frame_address(0), thread);
#endif
return (new_obj);
if (largep) return gc_alloc_large(nbytes, page_type);
int __attribute__((unused)) ret = mutex_acquire(&free_pages_lock);
gc_assert(ret);
ensure_region_closed(region, page_type);
// hold the lock after alloc_new_region if a cons page
int release = page_type != PAGE_TYPE_CONS;
new_obj = gc_alloc_new_region(nbytes, page_type, region, release);
region->free_pointer = (char*)new_obj + nbytes;
// addr_diff asserts that 'end' >= 'free_pointer'
int remaining = addr_diff(region->end_addr, region->free_pointer);
// Try to avoid the next Lisp -> C -> Lisp round-trip by possibly
// requesting yet another region.
if (page_type == PAGE_TYPE_CONS) {
if (remaining <= CONS_SIZE * N_WORD_BYTES) { // Refill now if <= 1 more cons to go
gc_close_region(region, page_type);
// Request > 2 words, forcing a new page to be claimed.
gc_alloc_new_region(4 * N_WORD_BYTES, page_type, region, 0); // don't release
}
ret = mutex_release(&free_pages_lock);
gc_assert(ret);
} else if (remaining <= 4 * N_WORD_BYTES
&& TryEnterCriticalSection(&free_pages_lock)) {
gc_close_region(region, page_type);
// Request > 4 words, forcing a new page to be claimed.
gc_alloc_new_region(6 * N_WORD_BYTES, page_type, region, 1); // do release
}
return new_obj;
}
// Code allocation is always serialized

View file

@ -73,13 +73,12 @@ static struct cons* private_cons_impl()
if (page >= 0 && (bytes_used = page_bytes_used(page)) < GENCGC_PAGE_BYTES) {
cons = (struct cons*)(page_address(page) + bytes_used);
} else {
page = alloc_start_page(PAGE_TYPE_UNBOXED, 0);
page = get_alloc_start_page(PAGE_TYPE_UNBOXED);
page_index_t last_page __attribute__((unused)) =
gc_find_freeish_pages(&page, GENCGC_PAGE_BYTES,
SINGLE_OBJECT_FLAG | PAGE_TYPE_UNBOXED,
GC_PRIVATE_CONS_GENERATION);
// See question about last_page in gc_alloc_large
set_alloc_start_page(PAGE_TYPE_UNBOXED, 0, page);
set_alloc_start_page(PAGE_TYPE_UNBOXED, page);
struct cons* page_header = (struct cons*)page_address(page);
if (PRIVATE_CONS_DEBUG)