From daec07c9f682967e2e1fc74494762e94bd3ed64b Mon Sep 17 00:00:00 2001 From: Georges Basile Stavracas Neto Date: Fri, 10 Mar 2023 13:19:30 -0300 Subject: [PATCH] search: Rework threading Search uses GcalTimeline, an object that is meant to be single threaded, in multple threads. That mostly worked okay because GcalTimeline mostly relies on GcalCalendarMonitor, which is heavily threaded. But, sometimes it failed. Rework that to be single-threaded, and asynchronous. Closes https://gitlab.gnome.org/GNOME/gnome-calendar/-/issues/981 Closes https://gitlab.gnome.org/GNOME/gnome-calendar/-/issues/987 Closes https://gitlab.gnome.org/GNOME/gnome-calendar/-/issues/988 --- src/search/gcal-search-engine.c | 102 +++++++++++--------------------- src/search/gcal-search-model.c | 76 ++++++++++++++++++------ src/search/gcal-search-model.h | 8 ++- 3 files changed, 101 insertions(+), 85 deletions(-) diff --git a/src/search/gcal-search-engine.c b/src/search/gcal-search-engine.c index 253daff8..47179ae6 100644 --- a/src/search/gcal-search-engine.c +++ b/src/search/gcal-search-engine.c @@ -22,20 +22,12 @@ #include "gcal-context.h" #include "gcal-date-time-utils.h" +#include "gcal-debug.h" #include "gcal-search-engine.h" #include "gcal-search-model.h" #include "gcal-timeline.h" #include "gcal-timeline-subscriber.h" -typedef struct -{ - GcalSearchEngine *engine; - gchar *query; - gint max_results; - GDateTime *range_start; - GDateTime *range_end; -} SearchData; - struct _GcalSearchEngine { GObject parent; @@ -57,59 +49,10 @@ enum static GParamSpec *properties [N_PROPS]; -/* - * Auxiliary methods - */ - -static void -search_data_free (gpointer data) -{ - SearchData *search_data = data; - - if (!data) - return; - - gcal_clear_date_time (&search_data->range_start); - gcal_clear_date_time (&search_data->range_end); - g_clear_pointer (&search_data->query, g_free); - g_clear_object (&search_data->engine); - g_slice_free (SearchData, data); -} - - /* * Callbacks */ -static void -search_func (GTask *task, - gpointer source_object, - gpointer task_data, - GCancellable *cancellable) -{ - g_autoptr (GcalSearchModel) model = NULL; - GcalSearchEngine *self; - SearchData *data; - - self = GCAL_SEARCH_ENGINE (source_object); - data = (SearchData*) task_data; - - model = gcal_search_model_new (cancellable, - data->max_results, - data->range_start, - data->range_end); - - gcal_timeline_set_filter (self->timeline, data->query); - gcal_timeline_add_subscriber (self->timeline, GCAL_TIMELINE_SUBSCRIBER (model)); - - gcal_search_model_wait_for_hits (model, cancellable); - - if (g_cancellable_is_cancelled (cancellable)) - g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_CANCELLED, "Cancelled"); - else - g_task_return_pointer (task, g_steal_pointer (&model), g_object_unref); -} - static void on_manager_calendar_added_cb (GcalManager *manager, GcalCalendar *calendar, @@ -130,6 +73,31 @@ on_manager_calendar_removed_cb (GcalManager *manager, gcal_timeline_remove_calendar (self->timeline, calendar); } +static void +search_model_hits_cb (GObject *source, + GAsyncResult *result, + gpointer data) +{ + g_autoptr (GError) error = NULL; + g_autoptr (GTask) task = data; + GcalSearchModel *search_model; + + GCAL_ENTRY; + + search_model = GCAL_SEARCH_MODEL (source); + gcal_search_model_wait_for_hits_finish (search_model, result, &error); + + if (error) + { + g_task_return_error (task, g_steal_pointer (&error)); + GCAL_RETURN (); + } + + g_task_return_pointer (task, g_object_ref (search_model), NULL); + + GCAL_EXIT; +} + /* * GObject overrides @@ -246,9 +214,11 @@ gcal_search_engine_search (GcalSearchEngine *self, GAsyncReadyCallback callback, gpointer user_data) { + g_autoptr (GcalSearchModel) model = NULL; + g_autoptr (GDateTime) range_start = NULL; + g_autoptr (GDateTime) range_end = NULL; g_autoptr (GDateTime) now = NULL; g_autoptr (GTask) task = NULL; - SearchData *data = NULL; GTimeZone *timezone; g_return_if_fail (GCAL_IS_SEARCH_ENGINE (self)); @@ -256,20 +226,18 @@ gcal_search_engine_search (GcalSearchEngine *self, timezone = gcal_context_get_timezone (self->context); now = g_date_time_new_now (timezone); + range_start = g_date_time_add_weeks (now, -1); + range_end = g_date_time_add_weeks (now, 3); + model = gcal_search_model_new (cancellable, max_results, range_start, range_end); - data = g_slice_new0 (SearchData); - data->engine = g_object_ref (self); - data->query = g_strdup (search_query); - data->max_results = max_results; - data->range_start = g_date_time_add_weeks (now, -1); - data->range_end = g_date_time_add_weeks (now, 3); + gcal_timeline_set_filter (self->timeline, search_query); + gcal_timeline_add_subscriber (self->timeline, GCAL_TIMELINE_SUBSCRIBER (model)); task = g_task_new (self, cancellable, callback, user_data); g_task_set_source_tag (task, gcal_search_engine_search); g_task_set_priority (task, G_PRIORITY_LOW); - g_task_set_task_data (task, data, (GDestroyNotify) search_data_free); - g_task_run_in_thread (task, search_func); + gcal_search_model_wait_for_hits (model, cancellable, search_model_hits_cb, g_object_ref (task)); } GListModel* diff --git a/src/search/gcal-search-model.c b/src/search/gcal-search-model.c index 892878b3..08cc0370 100644 --- a/src/search/gcal-search-model.c +++ b/src/search/gcal-search-model.c @@ -42,6 +42,9 @@ struct _GcalSearchModel GDateTime *range_end; GListModel *model; + + GTimer *timer; + guint idle_id; }; static void gcal_timeline_subscriber_interface_init (GcalTimelineSubscriberInterface *iface); @@ -58,6 +61,33 @@ G_DEFINE_TYPE_WITH_CODE (GcalSearchModel, gcal_search_model, G_TYPE_OBJECT, * Callbacks */ +static gboolean +check_for_search_hits_cb (gpointer user_data) +{ + GcalSearchModel *self; + GTask *task; + + task = G_TASK (user_data); + self = GCAL_SEARCH_MODEL (g_task_get_source_object (task)); + + if (g_task_return_error_if_cancelled (task)) + goto stop_idle; + + if (g_timer_elapsed (self->timer, NULL) >= WAIT_FOR_RESULTS_MS || + g_list_model_get_n_items (self->model) >= MIN_RESULTS) + { + g_task_return_boolean (task, TRUE); + goto stop_idle; + } + + return G_SOURCE_CONTINUE; + +stop_idle: + g_clear_pointer (&self->timer, g_timer_destroy); + self->idle_id = 0; + return G_SOURCE_REMOVE; +} + static void on_model_items_changed_cb (GListModel *model, guint position, @@ -208,6 +238,9 @@ gcal_search_model_finalize (GObject *object) { GcalSearchModel *self = (GcalSearchModel *)object; + g_assert (self->timer == NULL); + g_assert (self->idle_id == 0); + g_cancellable_cancel (self->cancellable); gcal_clear_date_time (&self->range_start); @@ -251,29 +284,38 @@ gcal_search_model_new (GCancellable *cancellable, } void -gcal_search_model_wait_for_hits (GcalSearchModel *self, - GCancellable *cancellable) +gcal_search_model_wait_for_hits (GcalSearchModel *self, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) { - g_autoptr (GMainContext) thread_context = NULL; - g_autoptr (GTimer) timer = NULL; + g_autoptr (GTask) task = NULL; GCAL_ENTRY; - g_return_if_fail (GCAL_IS_SEARCH_MODEL (self)); + g_assert (self->timer == NULL); + g_assert (self->idle_id == 0); - thread_context = g_main_context_ref_thread_default (); - timer = g_timer_new (); + task = g_task_new (self, cancellable, callback, user_data); + g_task_set_source_tag (task, gcal_search_model_wait_for_hits); + g_task_set_priority (task, G_PRIORITY_LOW); - g_timer_start (timer); - - while (g_list_model_get_n_items (self->model) < MIN_RESULTS && - g_timer_elapsed (timer, NULL) < WAIT_FOR_RESULTS_MS) - { - if (g_cancellable_is_cancelled (cancellable)) - break; - - g_main_context_iteration (thread_context, FALSE); - } + self->timer = g_timer_new (); + self->idle_id = g_idle_add_full (G_PRIORITY_LOW, + check_for_search_hits_cb, + g_object_ref (task), + g_object_unref); GCAL_EXIT; } + +gboolean +gcal_search_model_wait_for_hits_finish (GcalSearchModel *self, + GAsyncResult *result, + GError **error) +{ + g_return_val_if_fail (GCAL_IS_SEARCH_MODEL (self), FALSE); + g_return_val_if_fail (g_task_is_valid (result, self), FALSE); + + return g_task_propagate_boolean (G_TASK (result), error); +} diff --git a/src/search/gcal-search-model.h b/src/search/gcal-search-model.h index 97775834..8f040a00 100644 --- a/src/search/gcal-search-model.h +++ b/src/search/gcal-search-model.h @@ -33,6 +33,12 @@ GcalSearchModel* gcal_search_model_new (GCancellable GDateTime *range_end); void gcal_search_model_wait_for_hits (GcalSearchModel *self, - GCancellable *cancellable); + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); + +gboolean gcal_search_model_wait_for_hits_finish (GcalSearchModel *self, + GAsyncResult *result, + GError **error); G_END_DECLS