From da3c476ec9dfd1961d99d0066b226c62233bd37d Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Sat, 20 Jun 2015 22:22:07 +0300 Subject: [PATCH] Adjust places where profiling callbacks invoked (to match Mono GC) * alloc.c (GC_try_to_collect_inner): Move sending of GC_EVENT_START to the beginning of function (i.e. send right after GC_dont_gc check); move sending of GC_EVENT_END from GC_finish_collection to the end of this function (send it only if collection completed successfully); add TODO note about GC_EVENT_ABANDON notification. * alloc.c (start_world_inner): Remove. * alloc.c (GC_stopped_mark): Send GC_EVENT_PRE_STOP_WORLD, GC_EVENT_POST_STOP_WORLD, GC_EVENT_PRE_START_WORLD, GC_EVENT_POST_START_WORLD only if THREADS. * alloc.c (GC_stopped_mark): Send GC_EVENT_MARK_START before minimizing junk left in registers/stack (instead of after). * alloc.c (GC_stopped_mark): Do not send GC_EVENT_MARK_END in case of abandoned collection (add TODO note about GC_EVENT_MARK_ABANDON). * alloc.c (GC_stopped_mark): Send GC_EVENT_MARK_END after checking debugged objects for consistency (instead of before it). * darwin_stop_world.c (GC_suspend_thread_list): Send GC_EVENT_THREAD_SUSPENDED (in addition to that in GC_stop_world). * darwin_stop_world.c (GC_thread_resume): Move sending of GC_EVENT_THREAD_UNSUSPENDED from GC_start_world(). * pthread_stop_world.c (GC_suspend_all, GC_start_world): Do not send GC_EVENT_THREAD_SUSPENDED if pthread_kill/android_thread_kill failed. * pthread_stop_world.c (GC_start_world): Send GC_EVENT_THREAD_UNSUSPENDED after pthread_resume_np (in case of GC_OPENBSD_UTHREADS). * win32_threads.c (GC_stop_world): Move sending of GC_EVENT_THREAD_SUSPENDED to GC_suspend(). * win32_threads.c (GC_suspend, GC_start_world): Remove redundant cast to void* of THREAD_HANDLE(). --- alloc.c | 80 ++++++++++++++++++++++++++------------------ darwin_stop_world.c | 7 ++-- pthread_stop_world.c | 16 +++++---- win32_threads.c | 12 +++---- 4 files changed, 65 insertions(+), 50 deletions(-) diff --git a/alloc.c b/alloc.c index cac132f9..ed9cc4de 100644 --- a/alloc.c +++ b/alloc.c @@ -421,19 +421,21 @@ GC_INNER GC_bool GC_try_to_collect_inner(GC_stop_func stop_func) # endif ASSERT_CANCEL_DISABLED(); if (GC_dont_gc || (*stop_func)()) return FALSE; + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_START, NULL); if (GC_incremental && GC_collection_in_progress()) { GC_COND_LOG_PRINTF( "GC_try_to_collect_inner: finishing collection in progress\n"); /* Just finish collection already in progress. */ while(GC_collection_in_progress()) { - if ((*stop_func)()) return(FALSE); + if ((*stop_func)()) { + /* TODO: Notify GC_EVENT_ABANDON */ + return(FALSE); + } GC_collect_a_little_inner(1); } } GC_notify_full_gc(); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_START, NULL); - # ifndef SMALL_CONFIG if (GC_print_stats) { GET_TIME(start_time); @@ -453,6 +455,7 @@ GC_INNER GC_bool GC_try_to_collect_inner(GC_stop_func stop_func) if ((GC_find_leak || stop_func != GC_never_stop_func) && !GC_reclaim_all(stop_func, FALSE)) { /* Aborted. So far everything is still consistent. */ + /* TODO: Notify GC_EVENT_ABANDON */ return(FALSE); } GC_invalidate_mark_state(); /* Flush mark stack. */ @@ -470,6 +473,7 @@ GC_INNER GC_bool GC_try_to_collect_inner(GC_stop_func stop_func) GC_unpromote_black_lists(); } /* else we claim the world is already still consistent. We'll */ /* finish incrementally. */ + /* TODO: Notify GC_EVENT_ABANDON */ return(FALSE); } GC_finish_collection(); @@ -480,6 +484,8 @@ GC_INNER GC_bool GC_try_to_collect_inner(GC_stop_func stop_func) MS_TIME_DIFF(current_time,start_time)); } # endif + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_END, NULL); return(TRUE); } @@ -586,17 +592,6 @@ GC_API int GC_CALL GC_collect_a_little(void) # define COMMA_IF_USE_MUNMAP(x) /* empty */ #endif -GC_INLINE void start_world_inner(void) -{ - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_PRE_START_WORLD, NULL); - - START_WORLD(); - - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_POST_START_WORLD, NULL); -} - /* * Assumes lock is held. We stop the world and mark from all roots. * If stop_func() ever returns TRUE, we may fail and return FALSE. @@ -622,13 +617,17 @@ STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func) GET_TIME(start_time); # endif - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_PRE_STOP_WORLD, NULL); +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_PRE_STOP_WORLD, NULL); +# endif STOP_WORLD(); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_POST_STOP_WORLD, NULL); +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_POST_STOP_WORLD, NULL); +# endif # ifdef THREAD_LOCAL_ALLOC GC_world_stopped = TRUE; @@ -644,13 +643,13 @@ STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func) # endif /* Mark from all roots. */ + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_MARK_START, NULL); + /* Minimize junk left in my registers and on the stack */ GC_clear_a_few_frames(); GC_noop6(0,0,0,0,0,0); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_MARK_START, NULL); - GC_initiate_gc(); for (i = 0;;i++) { if ((*stop_func)()) { @@ -661,18 +660,24 @@ STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func) GC_world_stopped = FALSE; # endif - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_MARK_END, NULL); +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_PRE_START_WORLD, NULL); +# endif + + START_WORLD(); + +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_POST_START_WORLD, NULL); +# endif - start_world_inner(); + /* TODO: Notify GC_EVENT_MARK_ABANDON */ return(FALSE); } if (GC_mark_some(GC_approx_sp())) break; } - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_MARK_END, NULL); - GC_gc_no++; GC_DBGLOG_PRINTF("GC #%lu freed %ld bytes, heap %lu KiB" IF_USE_MUNMAP(" (+ %lu KiB unmapped)") "\n", @@ -684,11 +689,25 @@ STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func) if (GC_debugging_started) { (*GC_check_heap)(); } + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_MARK_END, NULL); # ifdef THREAD_LOCAL_ALLOC GC_world_stopped = FALSE; # endif - start_world_inner(); + +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_PRE_START_WORLD, NULL); +# endif + + START_WORLD(); + +# ifdef THREADS + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_POST_START_WORLD, NULL); +# endif + # ifndef SMALL_CONFIG if (GC_PRINT_STATS_FLAG) { unsigned long time_diff; @@ -994,9 +1013,6 @@ STATIC void GC_finish_collection(void) MS_TIME_DIFF(done_time,finalize_time)); } # endif - - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_END, NULL); } /* If stop_func == 0 then GC_default_stop_func is used instead. */ diff --git a/darwin_stop_world.c b/darwin_stop_world.c index aa58993e..1b6d0072 100644 --- a/darwin_stop_world.c +++ b/darwin_stop_world.c @@ -495,6 +495,8 @@ STATIC GC_bool GC_suspend_thread_list(thread_act_array_t act_list, int count, } if (!found) GC_mach_threads_count++; + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, (void *)thread); } return changed; } @@ -626,6 +628,8 @@ GC_INLINE void GC_thread_resume(thread_act_t thread) kern_result = thread_resume(thread); if (kern_result != KERN_SUCCESS) ABORT("thread_resume failed"); + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, (void *)thread); } /* Caller holds allocation lock, and has held it continuously since */ @@ -700,9 +704,6 @@ GC_INNER void GC_start_world(void) if ((p->flags & FINISHED) == 0 && !p->thread_blocked && p->stop_info.mach_thread != my_thread) GC_thread_resume(p->stop_info.mach_thread); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)p->stop_info.mach_thread); } } diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 2915a786..873b575f 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -516,15 +516,15 @@ STATIC int GC_suspend_all(void) thread_id = p -> kernel_id; result = android_thread_kill(thread_id, GC_sig_suspend); # endif - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, - (void *)thread_id); switch(result) { case ESRCH: /* Not really there anymore. Possible? */ n_live_threads--; break; case 0: + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, + (void *)thread_id); break; default: ABORT_ARG1("pthread_kill failed at suspend", @@ -844,6 +844,9 @@ GC_INNER void GC_start_world(void) # ifdef GC_OPENBSD_UTHREADS if (pthread_resume_np(p -> id) != 0) ABORT("pthread_resume_np failed"); + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, + (void *)p->id); # else # ifndef PLATFORM_ANDROID thread_id = p -> id; @@ -852,16 +855,15 @@ GC_INNER void GC_start_world(void) thread_id = p -> kernel_id; result = android_thread_kill(thread_id, GC_sig_thr_restart); # endif - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)thread_id); - switch(result) { case ESRCH: /* Not really there anymore. Possible? */ n_live_threads--; break; case 0: + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, + (void *)thread_id); break; default: ABORT_ARG1("pthread_kill failed at resume", diff --git a/win32_threads.c b/win32_threads.c index 84604c51..0da2d315 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -1182,6 +1182,8 @@ STATIC void GC_suspend(GC_thread t) # if defined(MPROTECT_VDB) AO_CLEAR(&GC_fault_handler_lock); # endif + if (GC_on_collection_event) + GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, THREAD_HANDLE(t)); } #if defined(GC_ASSERTIONS) && !defined(CYGWIN32) @@ -1234,9 +1236,6 @@ GC_INNER void GC_stop_world(void) if (t -> stack_base != 0 && t -> thread_blocked_sp == NULL && t -> id != thread_id) { GC_suspend((GC_thread)t); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, - (void *)THREAD_HANDLE(t)); } } } else @@ -1250,9 +1249,6 @@ GC_INNER void GC_stop_world(void) if (t -> stack_base != 0 && t -> thread_blocked_sp == NULL && !KNOWN_FINISHED(t) && t -> id != thread_id) { GC_suspend(t); - if (GC_on_collection_event) - GC_on_collection_event(GC_EVENT_THREAD_SUSPENDED, - (void *)THREAD_HANDLE(t)); } } } @@ -1288,7 +1284,7 @@ GC_INNER void GC_start_world(void) t -> suspended = FALSE; if (GC_on_collection_event) GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)THREAD_HANDLE(t)); + THREAD_HANDLE(t)); } } } else { @@ -1305,7 +1301,7 @@ GC_INNER void GC_start_world(void) t -> suspended = FALSE; if (GC_on_collection_event) GC_on_collection_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)THREAD_HANDLE(t)); + THREAD_HANDLE(t)); } } } -- 2.40.0