From 397b9991e1058c1e9e0f14a3a2ca686824020ed9 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Wed, 28 Sep 2011 13:48:17 +0400 Subject: [PATCH] Review 'disclaim' add-on partially; fix code in some places again --- finalized_mlc.c | 17 ++++++------ include/gc_disclaim.h | 11 ++++---- include/private/gc_priv.h | 10 +++---- include/private/thread_local_alloc.h | 4 +++ mark.c | 39 ++++++++++++++-------------- misc.c | 4 +-- reclaim.c | 22 +++++++++------- tests/disclaim_bench.c | 18 ++++++++----- thread_local_alloc.c | 6 +++++ 9 files changed, 70 insertions(+), 61 deletions(-) diff --git a/finalized_mlc.c b/finalized_mlc.c index da529b4e..8627435b 100644 --- a/finalized_mlc.c +++ b/finalized_mlc.c @@ -16,16 +16,16 @@ #ifdef ENABLE_DISCLAIM +#include "gc_disclaim.h" + #ifdef THREAD_LOCAL_ALLOC # include "private/thread_local_alloc.h" -#endif - -#include "gc_disclaim.h" +#else + STATIC ptr_t * GC_finalized_objfreelist = NULL; +#endif /* !THREAD_LOCAL_ALLOC */ STATIC int GC_finalized_kind = 0; -GC_INNER ptr_t * GC_finalized_objfreelist = NULL; - STATIC int GC_CALLBACK GC_finalized_disclaim(void *obj, void *cd) { struct GC_finalizer_closure *fc = *(void **)obj; @@ -44,20 +44,19 @@ STATIC int GC_CALLBACK GC_finalized_disclaim(void *obj, void *cd) return 0; } -static int done_init = 0; +static GC_bool done_init = FALSE; GC_API void GC_CALL GC_init_finalized_malloc(void) { DCL_LOCK_STATE; - GC_init(); // FIXME: Portable client should always do GC_INIT() itself - + GC_init(); /* In case it's not already done. */ LOCK(); if (done_init) { UNLOCK(); return; } - done_init = 1; + done_init = TRUE; GC_finalized_objfreelist = (ptr_t *)GC_new_free_list_inner(); GC_finalized_kind = GC_new_kind_inner((void **)GC_finalized_objfreelist, diff --git a/include/gc_disclaim.h b/include/gc_disclaim.h index f09bc9d6..0437f724 100644 --- a/include/gc_disclaim.h +++ b/include/gc_disclaim.h @@ -20,6 +20,9 @@ /* This API is defined only if the library has been suitably compiled */ /* (i.e. with ENABLE_DISCLAIM defined). */ +/* Prepare the object kind used for GC_finalized_malloc. */ +GC_API void GC_CALL GC_init_finalized_malloc(void); + /* Type of a disclaim call-back, always stored along with closure data */ /* passed as the second argument. */ typedef int (GC_CALLBACK * GC_disclaim_proc)(void * /*obj*/, void * /*cd*/); @@ -43,13 +46,9 @@ struct GC_finalizer_closure { /* Allocate "size" bytes which is finalized by "fc". This uses a */ /* dedicated object kind with a disclaim procedure, and is more */ -/* efficient than GC_register_finalizer and friends. You need to call */ -/* GC_init_finalized_malloc before using this. */ +/* efficient than GC_register_finalizer and friends. */ +/* GC_init_finalized_malloc must be called before using this. */ GC_API void *GC_CALL GC_finalized_malloc(size_t /*size*/, struct GC_finalizer_closure * /*fc*/); -/* Prepare the object kind used for GC_finalized_malloc. */ -GC_API void GC_CALL GC_init_finalized_malloc(void); - // FIXME: replace init with enable? - #endif diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 1273b65f..f8bd09d0 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -815,9 +815,9 @@ struct hblkhdr { /* Only set with USE_MUNMAP. */ # define FREE_BLK 4 /* Block is free, i.e. not in use. */ # ifdef ENABLE_DISCLAIM -# define HAS_DISCLAIM 8 +# define HAS_DISCLAIM 8 /* This kind has a callback on reclaim. */ -# define MARK_UNCONDITIONALLY 16 +# define MARK_UNCONDITIONALLY 0x10 /* Mark from all objects, marked or */ /* not. Used to mark objects needed by */ /* reclaim notifier. */ @@ -1243,7 +1243,7 @@ GC_EXTERN struct obj_kind { /* is reclaimed, but must also tolerate being */ /* called with object from freelist. Non-zero */ /* exit prevents object from being reclaimed. */ -# define OK_DISCLAIM_INITZ FALSE, NULL, NULL +# define OK_DISCLAIM_INITZ /* comma */, FALSE, NULL, NULL # else # define OK_DISCLAIM_INITZ /* empty */ # endif /* !ENABLE_DISCLAIM */ @@ -2053,10 +2053,6 @@ GC_EXTERN signed_word GC_bytes_found; GC_EXTERN ptr_t * GC_gcjobjfreelist; #endif -#ifdef ENABLE_DISCLAIM - GC_EXTERN ptr_t * GC_finalized_objfreelist; -#endif - #if defined(GWW_VDB) && defined(MPROTECT_VDB) GC_INNER GC_bool GC_gww_dirty_init(void); /* Defined in os_dep.c. Returns TRUE if GetWriteWatch is available. */ diff --git a/include/private/thread_local_alloc.h b/include/private/thread_local_alloc.h index 809fff55..3ebb922c 100644 --- a/include/private/thread_local_alloc.h +++ b/include/private/thread_local_alloc.h @@ -144,6 +144,10 @@ GC_INNER void GC_destroy_thread_local(GC_tlfs p); /* we take care of an individual thread freelist structure. */ GC_INNER void GC_mark_thread_local_fls_for(GC_tlfs p); +#ifdef ENABLE_DISCLAIM + GC_EXTERN ptr_t * GC_finalized_objfreelist; +#endif + extern #if defined(USE_COMPILER_TLS) __thread diff --git a/mark.c b/mark.c index 14e6afec..2ea24b9a 100644 --- a/mark.c +++ b/mark.c @@ -51,26 +51,26 @@ GC_INNER unsigned GC_n_mark_procs = GC_RESERVED_MARK_PROCS; /* It's done here, since we need to deal with mark descriptors. */ GC_INNER struct obj_kind GC_obj_kinds[MAXOBJKINDS] = { /* PTRFREE */ { &GC_aobjfreelist[0], 0 /* filled in dynamically */, - 0 | GC_DS_LENGTH, FALSE, FALSE, - OK_DISCLAIM_INITZ }, + 0 | GC_DS_LENGTH, FALSE, FALSE + /*, */ OK_DISCLAIM_INITZ }, /* NORMAL */ { &GC_objfreelist[0], 0, 0 | GC_DS_LENGTH, /* Adjusted in GC_init for EXTRA_BYTES */ - TRUE /* add length to descr */, TRUE, - OK_DISCLAIM_INITZ }, + TRUE /* add length to descr */, TRUE + /*, */ OK_DISCLAIM_INITZ }, /* UNCOLLECTABLE */ { &GC_uobjfreelist[0], 0, - 0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE, - OK_DISCLAIM_INITZ }, + 0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE + /*, */ OK_DISCLAIM_INITZ }, # ifdef ATOMIC_UNCOLLECTABLE /* AUNCOLLECTABLE */ { &GC_auobjfreelist[0], 0, - 0 | GC_DS_LENGTH, FALSE /* add length to descr */, FALSE, - OK_DISCLAIM_INITZ }, + 0 | GC_DS_LENGTH, FALSE /* add length to descr */, FALSE + /*, */ OK_DISCLAIM_INITZ }, # endif # ifdef STUBBORN_ALLOC /*STUBBORN*/ { (void **)&GC_sobjfreelist[0], 0, - 0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE, - OK_DISCLAIM_INITZ }, + 0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE + /*, */ OK_DISCLAIM_INITZ }, # endif }; @@ -1764,7 +1764,7 @@ STATIC void GC_push_marked(struct hblk *h, hdr *hhdr) } } -#ifdef MARK_UNCONDITIONALLY +#ifdef ENABLE_DISCLAIM /* Unconditionally mark from all objects which have not been reclaimed. */ /* This is useful in order to retain pointes which are reachable from */ /* the disclaim notifiers. */ @@ -1774,8 +1774,8 @@ STATIC void GC_push_marked(struct hblk *h, hdr *hhdr) /* first word. On the other hand, a reclaimed object is a members of */ /* free-lists, and thus contains a word-aligned next-pointer as the */ /* first word. */ -void GC_push_unconditionally(struct hblk *h, hdr *hhdr) -{ + STATIC void GC_push_unconditionally(struct hblk *h, hdr *hhdr) + { size_t sz = hhdr -> hb_sz; word descr = hhdr -> hb_descr; ptr_t p; @@ -1783,8 +1783,9 @@ void GC_push_unconditionally(struct hblk *h, hdr *hhdr) mse * GC_mark_stack_top_reg; mse * mark_stack_limit = GC_mark_stack_limit; - /* Shortcut */ - if ((0 | GC_DS_LENGTH) == descr) return; + if ((0 | GC_DS_LENGTH) == descr) + return; + GC_n_rescuing_pages++; GC_objects_are_marked = TRUE; if (sz > MAXOBJBYTES) @@ -1797,8 +1798,8 @@ void GC_push_unconditionally(struct hblk *h, hdr *hhdr) if ((*(GC_word *)p & 0x3) != 0) PUSH_OBJ(p, hhdr, GC_mark_stack_top_reg, mark_stack_limit); GC_mark_stack_top = GC_mark_stack_top_reg; -} -#endif + } +#endif /* ENABLE_DISCLAIM */ #ifndef GC_DISABLE_INCREMENTAL /* Test whether any page in the given block is dirty. */ @@ -1884,8 +1885,8 @@ STATIC struct hblk * GC_push_next_marked_uncollectable(struct hblk *h) GC_push_marked(h, hhdr); break; } -# ifdef MARK_UNCONDITIONALLY - if (hhdr -> hb_flags & MARK_UNCONDITIONALLY) { +# ifdef ENABLE_DISCLAIM + if ((hhdr -> hb_flags & MARK_UNCONDITIONALLY) != 0) { GC_push_unconditionally(h, hhdr); break; } diff --git a/misc.c b/misc.c index 02b39692..e825705e 100644 --- a/misc.c +++ b/misc.c @@ -1478,10 +1478,8 @@ GC_API unsigned GC_CALL GC_new_kind_inner(void **fl, GC_word descr, GC_obj_kinds[result].ok_descriptor = descr; GC_obj_kinds[result].ok_relocate_descr = adjust; GC_obj_kinds[result].ok_init = clear; -# ifdef MARK_UNCONDITIONALLY - GC_obj_kinds[result].ok_mark_unconditionally = FALSE; -# endif # ifdef ENABLE_DISCLAIM + GC_obj_kinds[result].ok_mark_unconditionally = FALSE; GC_obj_kinds[result].ok_disclaim_proc = 0; GC_obj_kinds[result].ok_disclaim_cd = NULL; # endif diff --git a/reclaim.c b/reclaim.c index e36aa7fc..f8ae8af9 100644 --- a/reclaim.c +++ b/reclaim.c @@ -45,8 +45,8 @@ STATIC unsigned GC_n_leaked = 0; GC_INNER GC_bool GC_have_errors = FALSE; -#if !defined(EAGER_SWEEP) && defined(MARK_UNCONDITIONALLY) -void GC_reclaim_unconditionally_marked(void); +#if !defined(EAGER_SWEEP) && defined(ENABLE_DISCLAIM) + STATIC void GC_reclaim_unconditionally_marked(void); #endif GC_INLINE void GC_add_leaked(ptr_t leaked) @@ -302,7 +302,7 @@ GC_INNER ptr_t GC_reclaim_generic(struct hblk * hbp, hdr *hhdr, size_t sz, GC_remove_protection(hbp, 1, (hhdr)->hb_descr == 0 /* Pointer-free? */); # endif # ifdef ENABLE_DISCLAIM - if (hhdr -> hb_flags & HAS_DISCLAIM) { + if ((hhdr -> hb_flags & HAS_DISCLAIM) != 0) { result = GC_disclaim_and_reclaim(hbp, hhdr, sz, list, count); } else # endif @@ -381,7 +381,8 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) if (report_if_found) { GC_add_leaked((ptr_t)hbp); } else { - size_t blocks = OBJ_SZ_TO_BLOCKS(sz); + size_t blocks; + # ifdef ENABLE_DISCLAIM if (EXPECT(hhdr->hb_flags & HAS_DISCLAIM, 0)) { struct obj_kind *ok = &GC_obj_kinds[hhdr->hb_obj_kind]; @@ -392,6 +393,7 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) } } # endif + blocks = OBJ_SZ_TO_BLOCKS(sz); if (blocks > 1) { GC_large_allocd_bytes -= blocks * HBLKSIZE; } @@ -646,7 +648,7 @@ GC_INNER void GC_start_reclaim(GC_bool report_if_found) /* This is a very stupid thing to do. We make it possible anyway, */ /* so that you can convince yourself that it really is very stupid. */ GC_reclaim_all((GC_stop_func)0, FALSE); -# elif defined(MARK_UNCONDITIONALLY) +# elif defined(ENABLE_DISCLAIM) /* However, make sure to clear reclaimable objects of kinds with */ /* unconditional marking enabled before we do any significant */ /* marking work. */ @@ -738,13 +740,13 @@ GC_INNER GC_bool GC_reclaim_all(GC_stop_func stop_func, GC_bool ignore_old) return(TRUE); } -#if !defined(EAGER_SWEEP) && defined(MARK_UNCONDITIONALLY) +#if !defined(EAGER_SWEEP) && defined(ENABLE_DISCLAIM) /* We do an eager sweep on heap blocks where unconditional marking has */ /* been enabled, so that any reclaimable objects have been reclaimed */ /* before we start marking. This is a simplified GC_reclaim_all */ /* restricted to kinds where ok_mark_unconditionally is true. */ -void GC_reclaim_unconditionally_marked(void) -{ + STATIC void GC_reclaim_unconditionally_marked(void) + { word sz; unsigned kind; hdr * hhdr; @@ -769,5 +771,5 @@ void GC_reclaim_unconditionally_marked(void) } } } -} -#endif + } +#endif /* !EAGER_SWEEP && ENABLE_DISCLAIM */ diff --git a/tests/disclaim_bench.c b/tests/disclaim_bench.c index 2bc37973..fb75d0d5 100644 --- a/tests/disclaim_bench.c +++ b/tests/disclaim_bench.c @@ -17,20 +17,21 @@ #include #include -#include // FIXME: It would be good not to use timing API by - // default (is it is not quite portable). - #include "atomic_ops.h" #include "gc_disclaim.h" #ifndef AO_HAVE_fetch_and_add1 + int main(void) { - printf("Skipping disclaim_bench since we don't have AO_fetch_and_add1.\n"); + printf("Skipping disclaim_bench test\n"); return 0; } + #else +#include + static AO_t free_count = 0; typedef struct testobj_s *testobj_t; @@ -95,19 +96,20 @@ int main(int argc, char **argv) keep_arr = GC_MALLOC(sizeof(void *)*KEEP_CNT); if (argc == 1) { +// FIXME: Remove this block and invoke this test 3 times from the script char *buf = GC_MALLOC(strlen(argv[0]) + 3); printf("\t\t\tfin. ratio time/s time/fin.\n"); fflush(stdout); for (i = 0; i < 3; ++i) { int st; sprintf(buf, "%s %d", argv[0], i); - st = system(buf); // FIXME: is this available on all targets? + st = system(buf); if (st != 0) return st; } return 0; } - if (argc == 2 && strcmp(argv[1], "--help") == 0) { + if (argc < 2 || (argc == 2 && strcmp(argv[1], "--help") == 0)) { fprintf(stderr, "Usage: %s FINALIZATION_MODEL\n" "\t0 -- original finalization\n" @@ -119,8 +121,10 @@ int main(int argc, char **argv) if (model < 0 || model > 2) exit(2); t = -(double)clock(); + // FIXME: clock() not portable + // FIXME: probably it's better to turn on timing only if some macro is on for (i = 0; i < ALLOC_CNT; ++i) { - int k = rand() % KEEP_CNT; + int k = rand() % KEEP_CNT; // FIXME: not protable - see dbg_mlc.c keep_arr[k] = testobj_new(model); } diff --git a/thread_local_alloc.c b/thread_local_alloc.c index 6122c885..2c76ad7e 100644 --- a/thread_local_alloc.c +++ b/thread_local_alloc.c @@ -32,6 +32,12 @@ GC_key_t GC_thread_key; static GC_bool keys_initialized; +#ifdef ENABLE_DISCLAIM + GC_INNER ptr_t * GC_finalized_objfreelist = NULL; + /* This variable is declared here to prevent linking of */ + /* finalized_mlc module unless the client uses the latter one. */ +#endif + /* Return a single nonempty freelist fl to the global one pointed to */ /* by gfl. */ -- 2.49.0