From 9f27f49e34c961fb577d259a037aa16222363462 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 20 May 2020 16:51:03 +0100 Subject: [PATCH] tests: Speed up the cancellable test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test added for #1841 spawned 100000 threads. That was fine on a desktop machine, but on a heavily loaded CI machine, it could result in large (and unpredictable) slowdowns, resulting in the test taking over 120s in about 1 in 5 runs, and hence failing that CI pipeline due to a timeout. When passing normally on CI, the test would take around 90s. Here’s a histogram of time per iteration on a failing (timed out) test run. Each iteration is one thread spawn: Iteration duration (µs) | Frequency ------------------------+---------- ≤100 | 0 100–200 | 30257 200–400 | 13696 400–800 | 1046 800–1000 | 123 1000–2000 | 583 2000–4000 | 3779 4000–8000 | 4972 8000–10000 | 1027 10000–20000 | 2610 20000–40000 | 650 40000–80000 | 86 80000–100000 | 10 100000–200000 | 2 >200000 | 0 There’s no actual need for the test to spawn 100000 threads, so rewrite it to reuse a single thread, and pass new data to that thread. Reverting the original commit (e4a690f5dd95) reproduces the failure on 100 out of 100 test runs with this commit applied, so the test still works. The test now takes 3s, rather than 11s, to run on my computer, and has passed when run with `meson test --repeat 1000 cancellable`. Signed-off-by: Philip Withnall --- gio/tests/cancellable.c | 58 +++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/gio/tests/cancellable.c b/gio/tests/cancellable.c index 002bdccca..e53ae6e7e 100644 --- a/gio/tests/cancellable.c +++ b/gio/tests/cancellable.c @@ -232,7 +232,8 @@ typedef struct { GCond cond; GMutex mutex; - GSource *cancellable_source; /* (owned) */ + gboolean thread_ready; + GAsyncQueue *cancellable_source_queue; /* (owned) (element-type GCancellableSource) */ } ThreadedDisposeData; static gboolean @@ -247,14 +248,18 @@ static gpointer threaded_dispose_thread_cb (gpointer user_data) { ThreadedDisposeData *data = user_data; + GSource *cancellable_source; - /* Synchronise with the main thread before trying to reproduce the race. */ g_mutex_lock (&data->mutex); + data->thread_ready = TRUE; g_cond_broadcast (&data->cond); g_mutex_unlock (&data->mutex); - /* Race with cancellation of the cancellable. */ - g_source_unref (data->cancellable_source); + while ((cancellable_source = g_async_queue_pop (data->cancellable_source_queue)) != (gpointer) 1) + { + /* Race with cancellation of the cancellable. */ + g_source_unref (cancellable_source); + } return NULL; } @@ -262,6 +267,8 @@ threaded_dispose_thread_cb (gpointer user_data) static void test_cancellable_source_threaded_dispose (void) { + ThreadedDisposeData data; + GThread *thread = NULL; guint i; g_test_summary ("Test a thread race between disposing of a GCancellableSource " @@ -269,12 +276,25 @@ test_cancellable_source_threaded_dispose (void) "to (in another thread)"); g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1841"); + /* Create a new thread and wait until it’s ready to execute. Each iteration of + * the test will pass it a new #GCancellableSource. */ + g_cond_init (&data.cond); + g_mutex_init (&data.mutex); + data.cancellable_source_queue = g_async_queue_new_full ((GDestroyNotify) g_source_unref); + data.thread_ready = FALSE; + + g_mutex_lock (&data.mutex); + thread = g_thread_new ("/cancellable-source/threaded-dispose", + threaded_dispose_thread_cb, &data); + + while (!data.thread_ready) + g_cond_wait (&data.cond, &data.mutex); + g_mutex_unlock (&data.mutex); + for (i = 0; i < 100000; i++) { GCancellable *cancellable = NULL; GSource *cancellable_source = NULL; - ThreadedDisposeData data; - GThread *thread = NULL; /* Create a cancellable and a cancellable source for it. For this test, * there’s no need to attach the source to a #GMainContext. */ @@ -282,26 +302,26 @@ test_cancellable_source_threaded_dispose (void) cancellable_source = g_cancellable_source_new (cancellable); g_source_set_callback (cancellable_source, G_SOURCE_FUNC (cancelled_cb), NULL, NULL); - /* Create a new thread and wait until it’s ready to execute before + /* Send it to the thread and wait until it’s ready to execute before * cancelling our cancellable. */ - g_cond_init (&data.cond); - g_mutex_init (&data.mutex); - data.cancellable_source = g_steal_pointer (&cancellable_source); - - g_mutex_lock (&data.mutex); - thread = g_thread_new ("/cancellable-source/threaded-dispose", - threaded_dispose_thread_cb, &data); - g_cond_wait (&data.cond, &data.mutex); - g_mutex_unlock (&data.mutex); + g_async_queue_push (data.cancellable_source_queue, g_steal_pointer (&cancellable_source)); /* Race with disposal of the cancellable source. */ g_cancellable_cancel (cancellable); - g_thread_join (g_steal_pointer (&thread)); - g_mutex_clear (&data.mutex); - g_cond_clear (&data.cond); g_object_unref (cancellable); } + + /* Indicate that the test has finished. Can’t use %NULL as #GAsyncQueue + * doesn’t allow that.*/ + g_async_queue_push (data.cancellable_source_queue, (gpointer) 1); + + g_thread_join (g_steal_pointer (&thread)); + + g_assert (g_async_queue_length (data.cancellable_source_queue) == 0); + g_async_queue_unref (data.cancellable_source_queue); + g_mutex_clear (&data.mutex); + g_cond_clear (&data.cond); } int