diff --git a/Makefile.in b/Makefile.in index edc50b4b..1be7d191 100644 --- a/Makefile.in +++ b/Makefile.in @@ -179,6 +179,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/prof_gdump.c \ $(srcroot)test/unit/prof_idump.c \ $(srcroot)test/unit/prof_reset.c \ + $(srcroot)test/unit/prof_tctx.c \ $(srcroot)test/unit/prof_thread_name.c \ $(srcroot)test/unit/ql.c \ $(srcroot)test/unit/qr.c \ diff --git a/include/jemalloc/internal/private_symbols.txt b/include/jemalloc/internal/private_symbols.txt index c85219a9..745220e3 100644 --- a/include/jemalloc/internal/private_symbols.txt +++ b/include/jemalloc/internal/private_symbols.txt @@ -369,6 +369,7 @@ prof_boot0 prof_boot1 prof_boot2 prof_bt_count +prof_cnt_all prof_dump_header prof_dump_open prof_free diff --git a/include/jemalloc/internal/prof_externs.h b/include/jemalloc/internal/prof_externs.h index 3f857145..76505f82 100644 --- a/include/jemalloc/internal/prof_externs.h +++ b/include/jemalloc/internal/prof_externs.h @@ -48,11 +48,12 @@ prof_tctx_t *prof_lookup(tsd_t *tsd, prof_bt_t *bt); #ifdef JEMALLOC_JET size_t prof_tdata_count(void); size_t prof_bt_count(void); -const prof_cnt_t *prof_cnt_all(void); typedef int (prof_dump_open_t)(bool, const char *); extern prof_dump_open_t *prof_dump_open; typedef bool (prof_dump_header_t)(tsdn_t *, bool, const prof_cnt_t *); extern prof_dump_header_t *prof_dump_header; +void prof_cnt_all(uint64_t *curobjs, uint64_t *curbytes, + uint64_t *accumobjs, uint64_t *accumbytes); #endif void prof_idump(tsdn_t *tsdn); bool prof_mdump(tsd_t *tsd, const char *filename); diff --git a/include/jemalloc/internal/prof_inlines.h b/include/jemalloc/internal/prof_inlines.h index a1ea7a32..394b7b37 100644 --- a/include/jemalloc/internal/prof_inlines.h +++ b/include/jemalloc/internal/prof_inlines.h @@ -194,30 +194,39 @@ prof_realloc(tsd_t *tsd, extent_t *extent, const void *ptr, size_t usize, } } - /* - * The following code must differentiate among eight possible cases, - * based on three boolean conditions. - */ sampled = ((uintptr_t)tctx > (uintptr_t)1U); old_sampled = ((uintptr_t)old_tctx > (uintptr_t)1U); moved = (ptr != old_ptr); - /* - * The following block must only execute if this is a non-moving - * reallocation, because for moving reallocation the old allocation will - * be deallocated via a separate call. - */ - if (unlikely(old_sampled) && !moved) - prof_free_sampled_object(tsd, old_usize, old_tctx); - if (unlikely(sampled)) { prof_malloc_sample_object(tsd_tsdn(tsd), extent, ptr, usize, tctx); } else if (moved) { prof_tctx_set(tsd_tsdn(tsd), extent, ptr, usize, (prof_tctx_t *)(uintptr_t)1U); - } else if (unlikely(old_sampled)) + } else if (unlikely(old_sampled)) { + /* + * prof_tctx_set() would work for the !moved case as well, but + * prof_tctx_reset() is slightly cheaper, and the proper thing + * to do here in the presence of explicit knowledge re: moved + * state. + */ prof_tctx_reset(tsd_tsdn(tsd), extent, ptr, tctx); + } else { + assert((uintptr_t)prof_tctx_get(tsd_tsdn(tsd), extent, ptr) == + (uintptr_t)1U); + } + + /* + * The prof_free_sampled_object() call must come after the + * prof_malloc_sample_object() call, because tctx and old_tctx may be + * the same, in which case reversing the call order could cause the tctx + * to be prematurely destroyed as a side effect of momentarily zeroed + * counters. + */ + if (unlikely(old_sampled)) { + prof_free_sampled_object(tsd, old_usize, old_tctx); + } } JEMALLOC_ALWAYS_INLINE void diff --git a/src/prof.c b/src/prof.c index 237cbb50..b161acfb 100644 --- a/src/prof.c +++ b/src/prof.c @@ -1530,95 +1530,178 @@ label_return: return (ret); } -static bool -prof_dump(tsd_t *tsd, bool propagate_err, const char *filename, bool leakcheck) +static void +prof_dump_prep(tsd_t *tsd, prof_tdata_t *tdata, + struct prof_tdata_merge_iter_arg_s *prof_tdata_merge_iter_arg, + struct prof_gctx_merge_iter_arg_s *prof_gctx_merge_iter_arg, + prof_gctx_tree_t *gctxs) { - prof_tdata_t *tdata; - struct prof_tdata_merge_iter_arg_s prof_tdata_merge_iter_arg; size_t tabind; union { prof_gctx_t *p; void *v; } gctx; - struct prof_gctx_merge_iter_arg_s prof_gctx_merge_iter_arg; - struct prof_gctx_dump_iter_arg_s prof_gctx_dump_iter_arg; - prof_gctx_tree_t gctxs; - cassert(config_prof); - - tdata = prof_tdata_get(tsd, true); - if (tdata == NULL) - return (true); - - malloc_mutex_lock(tsd_tsdn(tsd), &prof_dump_mtx); prof_enter(tsd, tdata); /* * Put gctx's in limbo and clear their counters in preparation for * summing. */ - gctx_tree_new(&gctxs); - for (tabind = 0; !ckh_iter(&bt2gctx, &tabind, NULL, &gctx.v);) - prof_dump_gctx_prep(tsd_tsdn(tsd), gctx.p, &gctxs); + gctx_tree_new(gctxs); + for (tabind = 0; !ckh_iter(&bt2gctx, &tabind, NULL, &gctx.v);) { + prof_dump_gctx_prep(tsd_tsdn(tsd), gctx.p, gctxs); + } /* * Iterate over tdatas, and for the non-expired ones snapshot their tctx * stats and merge them into the associated gctx's. */ - prof_tdata_merge_iter_arg.tsdn = tsd_tsdn(tsd); - memset(&prof_tdata_merge_iter_arg.cnt_all, 0, sizeof(prof_cnt_t)); + prof_tdata_merge_iter_arg->tsdn = tsd_tsdn(tsd); + memset(&prof_tdata_merge_iter_arg->cnt_all, 0, sizeof(prof_cnt_t)); malloc_mutex_lock(tsd_tsdn(tsd), &tdatas_mtx); tdata_tree_iter(&tdatas, NULL, prof_tdata_merge_iter, - (void *)&prof_tdata_merge_iter_arg); + (void *)prof_tdata_merge_iter_arg); malloc_mutex_unlock(tsd_tsdn(tsd), &tdatas_mtx); /* Merge tctx stats into gctx's. */ - prof_gctx_merge_iter_arg.tsdn = tsd_tsdn(tsd); - prof_gctx_merge_iter_arg.leak_ngctx = 0; - gctx_tree_iter(&gctxs, NULL, prof_gctx_merge_iter, - (void *)&prof_gctx_merge_iter_arg); + prof_gctx_merge_iter_arg->tsdn = tsd_tsdn(tsd); + prof_gctx_merge_iter_arg->leak_ngctx = 0; + gctx_tree_iter(gctxs, NULL, prof_gctx_merge_iter, + (void *)prof_gctx_merge_iter_arg); prof_leave(tsd, tdata); +} +static bool +prof_dump_file(tsd_t *tsd, bool propagate_err, const char *filename, + bool leakcheck, prof_tdata_t *tdata, + struct prof_tdata_merge_iter_arg_s *prof_tdata_merge_iter_arg, + struct prof_gctx_merge_iter_arg_s *prof_gctx_merge_iter_arg, + struct prof_gctx_dump_iter_arg_s *prof_gctx_dump_iter_arg, + prof_gctx_tree_t *gctxs) +{ /* Create dump file. */ - if ((prof_dump_fd = prof_dump_open(propagate_err, filename)) == -1) - goto label_open_close_error; + if ((prof_dump_fd = prof_dump_open(propagate_err, filename)) == -1) { + return true; + } /* Dump profile header. */ if (prof_dump_header(tsd_tsdn(tsd), propagate_err, - &prof_tdata_merge_iter_arg.cnt_all)) + &prof_tdata_merge_iter_arg->cnt_all)) { goto label_write_error; + } /* Dump per gctx profile stats. */ - prof_gctx_dump_iter_arg.tsdn = tsd_tsdn(tsd); - prof_gctx_dump_iter_arg.propagate_err = propagate_err; - if (gctx_tree_iter(&gctxs, NULL, prof_gctx_dump_iter, - (void *)&prof_gctx_dump_iter_arg) != NULL) + prof_gctx_dump_iter_arg->tsdn = tsd_tsdn(tsd); + prof_gctx_dump_iter_arg->propagate_err = propagate_err; + if (gctx_tree_iter(gctxs, NULL, prof_gctx_dump_iter, + (void *)prof_gctx_dump_iter_arg) != NULL) { goto label_write_error; + } /* Dump /proc//maps if possible. */ - if (prof_dump_maps(propagate_err)) + if (prof_dump_maps(propagate_err)) { goto label_write_error; + } - if (prof_dump_close(propagate_err)) - goto label_open_close_error; + if (prof_dump_close(propagate_err)) { + return true; + } + return false; +label_write_error: + prof_dump_close(propagate_err); + return true; +} + +static bool +prof_dump(tsd_t *tsd, bool propagate_err, const char *filename, bool leakcheck) +{ + prof_tdata_t *tdata; + struct prof_tdata_merge_iter_arg_s prof_tdata_merge_iter_arg; + struct prof_gctx_merge_iter_arg_s prof_gctx_merge_iter_arg; + struct prof_gctx_dump_iter_arg_s prof_gctx_dump_iter_arg; + prof_gctx_tree_t gctxs; + bool err; + + cassert(config_prof); + + tdata = prof_tdata_get(tsd, true); + if (tdata == NULL) { + return true; + } + + malloc_mutex_lock(tsd_tsdn(tsd), &prof_dump_mtx); + + prof_dump_prep(tsd, tdata, &prof_tdata_merge_iter_arg, + &prof_gctx_merge_iter_arg, &gctxs); + err = prof_dump_file(tsd, propagate_err, filename, leakcheck, tdata, + &prof_tdata_merge_iter_arg, &prof_gctx_merge_iter_arg, + &prof_gctx_dump_iter_arg, &gctxs); prof_gctx_finish(tsd, &gctxs); + malloc_mutex_unlock(tsd_tsdn(tsd), &prof_dump_mtx); + if (err) { + return true; + } + if (leakcheck) { prof_leakcheck(&prof_tdata_merge_iter_arg.cnt_all, prof_gctx_merge_iter_arg.leak_ngctx, filename); } - return (false); -label_write_error: - prof_dump_close(propagate_err); -label_open_close_error: - prof_gctx_finish(tsd, &gctxs); - malloc_mutex_unlock(tsd_tsdn(tsd), &prof_dump_mtx); - return (true); + return false; } +#ifdef JEMALLOC_JET +void +prof_cnt_all(uint64_t *curobjs, uint64_t *curbytes, uint64_t *accumobjs, + uint64_t *accumbytes) +{ + tsd_t *tsd; + prof_tdata_t *tdata; + struct prof_tdata_merge_iter_arg_s prof_tdata_merge_iter_arg; + struct prof_gctx_merge_iter_arg_s prof_gctx_merge_iter_arg; + prof_gctx_tree_t gctxs; + + tsd = tsd_fetch(); + tdata = prof_tdata_get(tsd, false); + if (tdata == NULL) { + if (curobjs != NULL) { + *curobjs = 0; + } + if (curbytes != NULL) { + *curbytes = 0; + } + if (accumobjs != NULL) { + *accumobjs = 0; + } + if (accumbytes != NULL) { + *accumbytes = 0; + } + return; + } + + prof_dump_prep(tsd, tdata, &prof_tdata_merge_iter_arg, + &prof_gctx_merge_iter_arg, &gctxs); + prof_gctx_finish(tsd, &gctxs); + + if (curobjs != NULL) { + *curobjs = prof_tdata_merge_iter_arg.cnt_all.curobjs; + } + if (curbytes != NULL) { + *curbytes = prof_tdata_merge_iter_arg.cnt_all.curbytes; + } + if (accumobjs != NULL) { + *accumobjs = prof_tdata_merge_iter_arg.cnt_all.accumobjs; + } + if (accumbytes != NULL) { + *accumbytes = prof_tdata_merge_iter_arg.cnt_all.accumbytes; + } +} +#endif + #define DUMP_FILENAME_BUFSIZE (PATH_MAX + 1) #define VSEQ_INVALID UINT64_C(0xffffffffffffffff) static void diff --git a/test/unit/prof_tctx.c b/test/unit/prof_tctx.c new file mode 100644 index 00000000..8f928ebf --- /dev/null +++ b/test/unit/prof_tctx.c @@ -0,0 +1,57 @@ +#include "test/jemalloc_test.h" + +#ifdef JEMALLOC_PROF +const char *malloc_conf = "prof:true,lg_prof_sample:0"; +#endif + +TEST_BEGIN(test_prof_realloc) +{ + tsdn_t *tsdn; + int flags; + void *p, *q; + extent_t *extent_p, *extent_q; + prof_tctx_t *tctx_p, *tctx_q; + uint64_t curobjs_0, curobjs_1, curobjs_2, curobjs_3; + + test_skip_if(!config_prof); + + tsdn = tsdn_fetch(); + flags = MALLOCX_TCACHE_NONE; + + prof_cnt_all(&curobjs_0, NULL, NULL, NULL); + p = mallocx(1024, flags); + assert_ptr_not_null(p, "Unexpected mallocx() failure"); + extent_p = iealloc(tsdn, p); + assert_ptr_not_null(extent_p, "Unexpected iealloc() failure"); + tctx_p = prof_tctx_get(tsdn, extent_p, p); + assert_ptr_ne(tctx_p, (prof_tctx_t *)(uintptr_t)1U, + "Expected valid tctx"); + prof_cnt_all(&curobjs_1, NULL, NULL, NULL); + assert_u64_eq(curobjs_0 + 1, curobjs_1, + "Allocation should have increased sample size"); + + q = rallocx(p, 2048, flags); + assert_ptr_ne(p, q, "Expected move"); + assert_ptr_not_null(p, "Unexpected rmallocx() failure"); + extent_q = iealloc(tsdn, q); + assert_ptr_not_null(extent_q, "Unexpected iealloc() failure"); + tctx_q = prof_tctx_get(tsdn, extent_q, q); + assert_ptr_ne(tctx_q, (prof_tctx_t *)(uintptr_t)1U, + "Expected valid tctx"); + prof_cnt_all(&curobjs_2, NULL, NULL, NULL); + assert_u64_eq(curobjs_1, curobjs_2, + "Reallocation should not have changed sample size"); + + dallocx(q, flags); + prof_cnt_all(&curobjs_3, NULL, NULL, NULL); + assert_u64_eq(curobjs_0, curobjs_3, + "Sample size should have returned to base level"); +} +TEST_END + +int +main(void) +{ + return test( + test_prof_realloc); +}