Skip to content

Commit

Permalink
[runtime] Implement thread info flags and get rid of tools threads. (m…
Browse files Browse the repository at this point in the history
…ono#7226)

* [acceptance-tests] Revert part of 1216780.

This didn't work as well as I'd hoped.

* [libgc] Export GC_start_blocking () and GC_end_blocking ().

* [libgc] Allow GC_start_blocking ()/GC_end_blocking () to be called unbalanced.

This makes things a bit easier in Mono.

* [libgc] Implement GC_start_blocking ()/GC_end_blocking () for Windows.

* [utils/event] Export the MonoOSEvent APIs for use in the profiler.

* [runtime] Implement thread info flags and get rid of tools threads.

The concept of a tools thread was originally a good solution to the problem of
some internal threads needing to be exempt from stop-the-world. Since they were
introduced, however, we've added more code in the profiler's internal threads
which require certain properties that don't hold true for tools threads. In
particular, we call into some APIs in the metadata layer that, while not
needing the managed heap, do need a domain to be set under certain conditions
(see mono#6188).

To avoid further complicating the semantics of tools thread by trying to set a
domain for them in a way that doesn't break things horribly, we'll instead do
away with the concept entirely. We already had mono_gc_set_skip_thread () to
flag a thread as being exempt from stop-the-world, which we used in thread pool
threads. This means we already have the infrastructure that is necessary for a
thread to be fully attached to the runtime as a managed thread while still
being exempt from stop-the-world.

So, we do the following:

1. Introduce a set of flags that can be set on a MonoThreadInfo at arbitrary
   points (although not in async context). These can indicate that the thread
   doesn't want to participate in stop-the-world, doesn't want to receive
   sampling signals, etc.
2. Change the thread iteration macros to allow skipping threads with specific
   flags set and use this where appropriate (e.g. SGen stop-the-world code).
3. Get rid of mono_gc_set_skip_thread () and make runtime code use the new
   mono_thread_info_set_flags () instead.
4. Switch all internal profiler threads to using mono_thread_attach () and
   mono_thread_detach (). Use mono_thread_info_set_flags () to disable
   stop-the-world and sampling signals for these threads immediately after they
   start.

With these changes, internal profiler threads are now fully attached threads
and can call any APIs in the runtime as long as they don't touch the managed
heap.

* [libgc] Add a note about how GC_start_blocking ()/GC_end_blocking () differ from upstream.
  • Loading branch information
alexrp authored and luhenry committed Feb 27, 2018
1 parent e8141fc commit a5da7b2
Show file tree
Hide file tree
Showing 25 changed files with 439 additions and 204 deletions.
21 changes: 10 additions & 11 deletions acceptance-tests/profiler-stress/runner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ sealed class TestResult {
public int? ExitCode { get; set; }
public string StandardOutput { get; set; }
public string StandardError { get; set; }
public string CombinedOutput { get; set; }
}

static class Program {
Expand Down Expand Up @@ -191,22 +190,17 @@ static int Main ()

var stdout = new StringBuilder ();
var stderr = new StringBuilder ();
var combined = new StringBuilder ();

proc.OutputDataReceived += (sender, args) => {
if (args.Data != null)
lock (result) {
lock (result)
stdout.AppendLine (args.Data);
combined.AppendLine (args.Data);
}
};

proc.ErrorDataReceived += (sender, args) => {
if (args.Data != null)
lock (result) {
lock (result)
stderr.AppendLine (args.Data);
combined.AppendLine (args.Data);
}
};

result.Stopwatch.Start ();
Expand All @@ -233,7 +227,6 @@ static int Main ()
lock (result) {
result.StandardOutput = stdout.ToString ();
result.StandardError = stderr.ToString ();
result.CombinedOutput = combined.ToString ();
}
}

Expand All @@ -245,10 +238,16 @@ static int Main ()

if (result.ExitCode != 0) {
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine ("===== stdout + stderr =====");
Console.WriteLine ("===== stdout =====");
Console.ResetColor ();

Console.WriteLine (result.CombinedOutput);
Console.WriteLine (result.StandardOutput);

Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine ("===== stderr =====");
Console.ResetColor ();

Console.WriteLine (result.StandardError);
}

results.Add (result);
Expand Down
12 changes: 12 additions & 0 deletions libgc/include/gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,18 @@ typedef GC_PTR (*GC_fn_type) GC_PROTO((GC_PTR client_data));
GC_API GC_PTR GC_call_with_alloc_lock
GC_PROTO((GC_fn_type fn, GC_PTR client_data));

/*
* These are similar to GC_do_blocking () in upstream bdwgc. The design is
* simpler in that there is no distinction between active and inactive stack
* frames; instead, while a thread is in blocking state, it promises to not
* interact with the GC at all, and to not keep any pointers to GC memory
* around from before entering blocking state. Additionally, these can be
* called unbalanced (they simply set a flag internally).
*/
GC_API void GC_start_blocking GC_PROTO((void));

GC_API void GC_end_blocking GC_PROTO((void));

/* The following routines are primarily intended for use with a */
/* preprocessor which inserts calls to check C pointer arithmetic. */
/* They indicate failure by invoking the corresponding _print_proc. */
Expand Down
2 changes: 0 additions & 2 deletions libgc/pthread_support.c
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,6 @@ void GC_start_blocking(void) {
GC_thread me;
LOCK();
me = GC_lookup_thread(pthread_self());
GC_ASSERT(!(me -> thread_blocked));
# ifdef SPARC
me -> stop_info.stack_ptr = (ptr_t)GC_save_regs_in_stack();
# else
Expand Down Expand Up @@ -1273,7 +1272,6 @@ void GC_end_blocking(void) {
GC_thread me;
LOCK(); /* This will block if the world is stopped. */
me = GC_lookup_thread(pthread_self());
GC_ASSERT(me -> thread_blocked);
me -> thread_blocked = FALSE;
UNLOCK();
}
Expand Down
48 changes: 47 additions & 1 deletion libgc/win32_threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ struct GC_thread_Rep {
/* 0 ==> entry not valid. */
/* !in_use ==> stack_base == 0 */
GC_bool suspended;
GC_bool thread_blocked;

# ifdef CYGWIN32
void *status; /* hold exit value until join in case it's a pointer */
Expand Down Expand Up @@ -174,6 +175,7 @@ static GC_thread GC_new_thread(void) {
/* the world, wait here. Hopefully this can't happen on any */
/* systems that don't allow us to block here. */
while (GC_please_stop) Sleep(20);
GC_ASSERT(!thread_table[i]->thread_blocked);
return thread_table + i;
}

Expand Down Expand Up @@ -223,7 +225,6 @@ static void GC_delete_thread(DWORD thread_id) {
}
}


#ifdef CYGWIN32

/* Return a GC_thread corresponding to a given pthread_t. */
Expand All @@ -245,6 +246,20 @@ static GC_thread GC_lookup_thread(pthread_t id)
return thread_table + i;
}

#else

static GC_thread GC_lookup_thread(DWORD id)
{
int i;
LONG max = GC_get_max_thread_index();

for (i = 0; i <= max; i++)
if (thread_table[i].in_use && thread_table[i].id == id)
return &thread_table[i];

return NULL;
}

#endif /* CYGWIN32 */

void GC_push_thread_structures GC_PROTO((void))
Expand All @@ -264,6 +279,35 @@ void GC_push_thread_structures GC_PROTO((void))
# endif
}

/* Wrappers for functions that are likely to block for an appreciable */
/* length of time. Must be called in pairs, if at all. */
/* Nothing much beyond the system call itself should be executed */
/* between these. */

void GC_start_blocking(void) {
GC_thread me;
LOCK();
#ifdef CYGWIN32
me = GC_lookup_thread(pthread_self());
#else
me = GC_lookup_thread(GetCurrentThreadId());
#endif
me->thread_blocked = TRUE;
UNLOCK();
}

void GC_end_blocking(void) {
GC_thread me;
LOCK(); /* This will block if the world is stopped. */
#ifdef CYGWIN32
me = GC_lookup_thread(pthread_self());
#else
me = GC_lookup_thread(GetCurrentThreadId());
#endif
me->thread_blocked = FALSE;
UNLOCK();
}

/* Defined in misc.c */
extern CRITICAL_SECTION GC_write_cs;

Expand All @@ -281,6 +325,8 @@ void GC_stop_world()
for (i = 0; i <= GC_get_max_thread_index(); i++)
if (thread_table[i].stack_base != 0
&& thread_table[i].id != thread_id) {
if (thread_table [i].thread_blocked)
continue;
# ifdef MSWINCE
/* SuspendThread will fail if thread is running kernel code */
while (SuspendThread(thread_table[i].handle) == (DWORD)-1)
Expand Down
19 changes: 17 additions & 2 deletions mono/metadata/boehm-gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ static void
mono_push_other_roots (void)
{
g_hash_table_foreach (roots, push_root, NULL);
FOREACH_THREAD (info) {
FOREACH_THREAD_EXCLUDE (info, MONO_THREAD_INFO_FLAGS_NO_GC) {
HandleStack* stack = (HandleStack*)info->handle_stack;
if (stack)
push_handle_stack (stack);
Expand Down Expand Up @@ -1478,7 +1478,22 @@ mono_gc_set_stack_end (void *stack_end)
{
}

void mono_gc_set_skip_thread (gboolean value)
void
mono_gc_skip_thread_changing (gboolean skip)
{
/*
* Unlike SGen, Boehm doesn't respect our thread info flags. We need to
* inform Boehm manually to skip/not skip the current thread.
*/

if (skip)
GC_start_blocking ();
else
GC_end_blocking ();
}

void
mono_gc_skip_thread_changed (gboolean skip)
{
}

Expand Down
4 changes: 3 additions & 1 deletion mono/metadata/gc-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ gboolean mono_gc_card_table_nursery_check (void);

void* mono_gc_get_nursery (int *shift_bits, size_t *size);

void mono_gc_set_skip_thread (gboolean skip);
// Don't use directly; set/unset MONO_THREAD_INFO_FLAGS_NO_GC instead.
void mono_gc_skip_thread_changing (gboolean skip);
void mono_gc_skip_thread_changed (gboolean skip);

#ifndef HOST_WIN32
int mono_gc_pthread_create (pthread_t *new_thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
Expand Down
4 changes: 2 additions & 2 deletions mono/metadata/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -893,15 +893,15 @@ finalizer_thread (gpointer unused)
*/

g_assert (mono_domain_get () == mono_get_root_domain ());
mono_gc_set_skip_thread (TRUE);
mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NO_GC);

if (wait) {
/* An alertable wait is required so this thread can be suspended on windows */
mono_coop_sem_wait (&finalizer_sem, MONO_SEM_FLAGS_ALERTABLE);
}
wait = TRUE;

mono_gc_set_skip_thread (FALSE);
mono_thread_info_set_flags (MONO_THREAD_INFO_FLAGS_NONE);

mono_runtime_do_background_work ();

Expand Down
9 changes: 8 additions & 1 deletion mono/metadata/null-gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,14 @@ mono_gc_pthread_create (pthread_t *new_thread, const pthread_attr_t *attr, void
}
#endif

void mono_gc_set_skip_thread (gboolean value)
void
mono_gc_skip_thread_changing (gboolean skip)
{
// No STW, nothing needs to be done.
}

void
mono_gc_skip_thread_changed (gboolean skip)
{
}

Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/object-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -1849,7 +1849,7 @@ mono_ldstr_checked (MonoDomain *domain, MonoImage *image, uint32_t str_index, Mo
MonoString*
mono_string_new_len_checked (MonoDomain *domain, const char *text, guint length, MonoError *error);

MonoString*
MONO_PROFILER_API MonoString*
mono_string_new_checked (MonoDomain *domain, const char *text, MonoError *merror);

MonoString*
Expand Down
9 changes: 0 additions & 9 deletions mono/metadata/sgen-client-mono.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@ struct _SgenClientThreadInfo {
gboolean skip, suspend_done;
volatile int in_critical_region;

/*
This is set the argument of mono_gc_set_skip_thread.
A thread that knowingly holds no managed state can call this
function around blocking loops to reduce the GC burden by not
been scanned.
*/
gboolean gc_disabled;

#ifdef SGEN_POSIX_STW
/* This is -1 until the first suspend. */
int signal;
Expand Down
36 changes: 20 additions & 16 deletions mono/metadata/sgen-mono.c
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ mono_gc_clear_domain (MonoDomain * domain)

sgen_clear_nursery_fragments ();

FOREACH_THREAD (info) {
FOREACH_THREAD_ALL (info) {
mono_handle_stack_free_domain ((HandleStack*)info->client_info.info.handle_stack, domain);
} FOREACH_THREAD_END

Expand Down Expand Up @@ -2074,13 +2074,11 @@ static void
report_stack_roots (void)
{
GCRootReport report = {0};
FOREACH_THREAD (info) {
FOREACH_THREAD_EXCLUDE (info, MONO_THREAD_INFO_FLAGS_NO_GC) {
void *aligned_stack_start;

if (info->client_info.skip) {
continue;
} else if (info->client_info.gc_disabled) {
continue;
} else if (!mono_thread_info_is_live (info)) {
continue;
} else if (!info->client_info.stack_start) {
Expand Down Expand Up @@ -2450,7 +2448,7 @@ sgen_client_thread_attach (SgenThreadInfo* info)
{
mono_tls_set_sgen_thread_info (info);

info->client_info.skip = 0;
info->client_info.skip = FALSE;

info->client_info.stack_start = NULL;

Expand Down Expand Up @@ -2502,23 +2500,32 @@ sgen_client_thread_detach_with_lock (SgenThreadInfo *p)
}

void
mono_gc_set_skip_thread (gboolean skip)
mono_gc_skip_thread_changing (gboolean skip)
{
SgenThreadInfo *info = mono_thread_info_current ();

/*
* SGen's STW will respect the thread info flags, but we do need to take
* the GC lock when changing them. If we don't do this, SGen might end up
* trying to resume a thread that wasn't suspended because it had
* MONO_THREAD_INFO_FLAGS_NO_GC set when STW began.
*/
LOCK_GC;
info->client_info.gc_disabled = skip;
UNLOCK_GC;

if (skip) {
/* If we skip scanning a thread with a non-empty handle stack, we may move an
/*
* If we skip scanning a thread with a non-empty handle stack, we may move an
* object but fail to update the reference in the handle.
*/
HandleStack *stack = info->client_info.info.handle_stack;
HandleStack *stack = mono_thread_info_current ()->client_info.info.handle_stack;
g_assert (stack == NULL || mono_handle_stack_is_empty (stack));
}
}

void
mono_gc_skip_thread_changed (gboolean skip)
{
UNLOCK_GC;
}

gboolean
mono_gc_thread_in_critical_region (SgenThreadInfo *info)
{
Expand Down Expand Up @@ -2591,16 +2598,13 @@ sgen_client_scan_thread_data (void *start_nursery, void *end_nursery, gboolean p
return;
#endif

FOREACH_THREAD (info) {
FOREACH_THREAD_EXCLUDE (info, MONO_THREAD_INFO_FLAGS_NO_GC) {
int skip_reason = 0;
void *aligned_stack_start;

if (info->client_info.skip) {
SGEN_LOG (3, "Skipping dead thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start);
skip_reason = 1;
} else if (info->client_info.gc_disabled) {
SGEN_LOG (3, "GC disabled for thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start);
skip_reason = 2;
} else if (!mono_thread_info_is_live (info)) {
SGEN_LOG (3, "Skipping non-running thread %p, range: %p-%p, size: %zd (state %x)", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start, info->client_info.info.thread_state);
skip_reason = 3;
Expand Down
Loading

0 comments on commit a5da7b2

Please sign in to comment.