From c0a094e4734a1a760681e75565af10bf86b68bee Mon Sep 17 00:00:00 2001 From: Přemysl Eric Janouch Date: Sat, 16 Oct 2021 08:27:42 +0200 Subject: sdgui: load dictionaries in parallel, as sdtui did Also, resolve some use-after-frees in GTK+. --- src/sdgui.c | 76 +++++++++++-------------------- src/sdtui.c | 149 ++++++++++++++++-------------------------------------------- src/utils.c | 101 ++++++++++++++++++++++++++++++++++++---- src/utils.h | 21 ++++++++- 4 files changed, 175 insertions(+), 172 deletions(-) diff --git a/src/sdgui.c b/src/sdgui.c index cc2a042..b1b1329 100644 --- a/src/sdgui.c +++ b/src/sdgui.c @@ -26,15 +26,6 @@ #include "utils.h" #include "stardict-view.h" -typedef struct dictionary Dictionary; - -struct dictionary -{ - const gchar *filename; ///< Filename - StardictDict *dict; ///< Stardict dictionary data - gchar *name; ///< Name to show -}; - static struct { GtkWidget *window; ///< Top-level window @@ -43,8 +34,7 @@ static struct GtkWidget *view; ///< Entries view gint dictionary; ///< Index of the current dictionary - Dictionary *dictionaries; ///< All open dictionaries - gsize dictionaries_len; ///< Total number of dictionaries + GPtrArray *dictionaries; ///< All open dictionaries gboolean watch_selection; ///< Following X11 PRIMARY? } @@ -52,31 +42,15 @@ g; // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -static gboolean -dictionary_load (Dictionary *self, GError **e) -{ - if (!(self->dict = stardict_dict_new (self->filename, e))) - return FALSE; - - if (!self->name) - { - self->name = g_strdup (stardict_info_get_book_name - (stardict_dict_get_info (self->dict))); - } - return TRUE; -} - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - static void init (gchar **filenames) { - while (filenames[g.dictionaries_len]) - g.dictionaries_len++; - - g.dictionaries = g_malloc0_n (sizeof *g.dictionaries, g.dictionaries_len); - for (gsize i = 0; i < g.dictionaries_len; i++) - g.dictionaries[i].filename = filenames[i]; + for (gsize i = 0; filenames[i]; i++) + { + Dictionary *dict = g_malloc0 (sizeof *dict); + dict->filename = g_strdup (filenames[i]); + g_ptr_array_add (g.dictionaries, dict); + } } // TODO: try to deduplicate, similar to app_load_config_values() @@ -84,19 +58,21 @@ static gboolean init_from_key_file (GKeyFile *kf, GError **error) { const gchar *dictionaries = "Dictionaries"; - gchar **names = - g_key_file_get_keys (kf, dictionaries, &g.dictionaries_len, NULL); + gchar **names = g_key_file_get_keys (kf, dictionaries, NULL, NULL); if (!names) return TRUE; - g.dictionaries = g_malloc0_n (sizeof *g.dictionaries, g.dictionaries_len); - for (gsize i = 0; i < g.dictionaries_len; i++) - g.dictionaries[i].name = names[i]; + for (gsize i = 0; names[i]; i++) + { + Dictionary *dict = g_malloc0 (sizeof *dict); + dict->name = names[i]; + g_ptr_array_add (g.dictionaries, dict); + } g_free (names); - for (gsize i = 0; i < g.dictionaries_len; i++) + for (gsize i = 0; i < g.dictionaries->len; i++) { - Dictionary *dict = &g.dictionaries[i]; + Dictionary *dict = g_ptr_array_index (g.dictionaries, i); gchar *path = g_key_file_get_string (kf, dictionaries, dict->name, error); if (!path) @@ -142,7 +118,7 @@ search (Dictionary *dict) static void on_changed (G_GNUC_UNUSED GtkWidget *widget, G_GNUC_UNUSED gpointer data) { - search (&g.dictionaries[g.dictionary]); + search (g_ptr_array_index (g.dictionaries, g.dictionary)); } static void @@ -178,7 +154,7 @@ on_switch_page (G_GNUC_UNUSED GtkWidget *widget, G_GNUC_UNUSED GtkWidget *page, guint page_num, G_GNUC_UNUSED gpointer data) { g.dictionary = page_num; - search (&g.dictionaries[g.dictionary]); + search (g_ptr_array_index (g.dictionaries, g.dictionary)); } static gboolean @@ -256,17 +232,19 @@ main (int argc, char *argv[]) return 1; } + g.dictionaries = + g_ptr_array_new_with_free_func ((GDestroyNotify) dictionary_destroy); if (filenames) init (filenames); else if (!init_from_config (&error) && error) die_with_dialog (error->message); + g_strfreev (filenames); - for (gsize i = 0; i < g.dictionaries_len; i++) - if (!dictionary_load (&g.dictionaries[i], &error)) - die_with_dialog (error->message); - if (!g.dictionaries_len) + if (!g.dictionaries->len) die_with_dialog (_("No dictionaries found either in " "the configuration or on the command line")); + if (!load_dictionaries (g.dictionaries, &error)) + die_with_dialog (error->message); // Some Adwaita stupidity const char *style = "notebook header tab { padding: 2px 8px; margin: 0; }"; @@ -331,9 +309,9 @@ main (int argc, char *argv[]) g.view = stardict_view_new (); gtk_box_pack_end (GTK_BOX (superbox), g.view, TRUE, TRUE, 0); - for (gsize i = 0; i < g.dictionaries_len; i++) + for (gsize i = 0; i < g.dictionaries->len; i++) { - Dictionary *dict = &g.dictionaries[i]; + Dictionary *dict = g_ptr_array_index (g.dictionaries, i); GtkWidget *dummy = gtk_box_new (GTK_ORIENTATION_VERTICAL, 0); GtkWidget *label = gtk_label_new (dict->name); gtk_notebook_append_page (GTK_NOTEBOOK (g.notebook), dummy, label); @@ -346,7 +324,5 @@ main (int argc, char *argv[]) gtk_widget_grab_focus (g.entry); gtk_widget_show_all (g.window); gtk_main (); - - g_strfreev (filenames); return 0; } diff --git a/src/sdtui.c b/src/sdtui.c index caf08e6..f7cdb02 100644 --- a/src/sdtui.c +++ b/src/sdtui.c @@ -125,7 +125,7 @@ struct attrs /// Data relating to one entry within the dictionary. typedef struct view_entry ViewEntry; /// Data relating to a dictionary file. -typedef struct dictionary Dictionary; +typedef struct app_dictionary AppDictionary; /// Encloses application data. typedef struct application Application; @@ -136,12 +136,10 @@ struct view_entry GPtrArray * formatting; ///< chtype * or NULL per definition }; -struct dictionary +struct app_dictionary { - gchar * name; ///< Visible identifier - gsize name_width; ///< Visible width of the name - gchar * filename; ///< Path to the dictionary - StardictDict * dict; ///< Dictionary + Dictionary super; ///< Superclass + gsize name_width; ///< Visible width of the name }; struct application @@ -153,7 +151,7 @@ struct application gboolean locale_is_utf8; ///< The locale is Unicode gboolean focused; ///< Whether the terminal has focus - GArray * dictionaries; ///< All loaded dictionaries + GPtrArray * dictionaries; ///< All loaded AppDictionaries StardictDict * dict; ///< The current dictionary guint show_help : 1; ///< Whether help can be shown @@ -352,42 +350,6 @@ view_entry_free (ViewEntry *ve) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -static gboolean -dictionary_load (Dictionary *self, Application *app, GError **e) -{ - if (!(self->dict = stardict_dict_new (self->filename, e))) - return FALSE; - - if (!self->name) - { - self->name = g_strdup (stardict_info_get_book_name - (stardict_dict_get_info (self->dict))); - } - - // Add some padding for decorative purposes - gchar *tmp = g_strdup_printf (" %s ", self->name); - g_free (self->name); - self->name = tmp; - - gunichar *ucs4 = g_utf8_to_ucs4_fast (self->name, -1, NULL); - for (gunichar *it = ucs4; *it; it++) - self->name_width += app_char_width (app, *it); - g_free (ucs4); - return TRUE; -} - -static void -dictionary_free (Dictionary *self) -{ - g_free (self->name); - g_free (self->filename); - - if (self->dict) - g_object_unref (self->dict); -} - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - /// Reload view items. static void app_reload_view (Application *self) @@ -491,8 +453,10 @@ app_load_config_values (Application *self, GKeyFile *kf) else resolved = path; - Dictionary dict = { .name = g_strdup (*it), .filename = resolved }; - g_array_append_val (self->dictionaries, dict); + AppDictionary *dict = g_malloc0 (sizeof *dict); + dict->super.name = g_strdup (*it); + dict->super.filename = resolved; + g_ptr_array_add (self->dictionaries, dict); } g_strfreev (names); } @@ -521,67 +485,29 @@ app_init_attrs (Application *self) #undef XX } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - static gboolean app_load_dictionaries (Application *self, GError **e) { - for (guint i = 0; i < self->dictionaries->len; i++) - if (!dictionary_load (&g_array_index (self->dictionaries, - Dictionary, i), self, e)) - return FALSE; - return TRUE; -} - -// Parallelize dictionary loading if possible, because of collation reindexing -#if GLIB_CHECK_VERSION (2, 36, 0) -struct load_ctx -{ - Application *self; ///< Application context - GAsyncQueue *error_queue; ///< Errors -}; - -static void -app_load_worker (gpointer data, gpointer user_data) -{ - struct load_ctx *ctx = user_data; - GError *e = NULL; - dictionary_load (&g_array_index (ctx->self->dictionaries, Dictionary, - GPOINTER_TO_UINT (data) - 1), ctx->self, &e); - if (e) - g_async_queue_push (ctx->error_queue, e); -} + if (!load_dictionaries (self->dictionaries, e)) + return FALSE; -static gboolean -app_load_dictionaries_parallel (Application *self, GError **e) -{ - struct load_ctx ctx; - GThreadPool *pool = g_thread_pool_new (app_load_worker, &ctx, - g_get_num_processors (), TRUE, NULL); - if G_UNLIKELY (!g_thread_pool_get_num_threads (pool)) + for (gsize i = 0; i < self->dictionaries->len; i++) { - g_thread_pool_free (pool, TRUE, TRUE); - return app_load_dictionaries (self, e); - } - - ctx.self = self; - ctx.error_queue = g_async_queue_new_full ((GDestroyNotify) g_error_free); - for (guint i = 0; i < self->dictionaries->len; i++) - g_thread_pool_push (pool, GUINT_TO_POINTER (i + 1), NULL); + AppDictionary *dict = g_ptr_array_index (self->dictionaries, i); - g_thread_pool_free (pool, FALSE, TRUE); + // Add some padding for decorative purposes + gchar *tmp = g_strdup_printf (" %s ", dict->super.name); + g_free (dict->super.name); + dict->super.name = tmp; - gboolean result = TRUE; - if ((*e = g_async_queue_try_pop (ctx.error_queue))) - result = FALSE; - - g_async_queue_unref (ctx.error_queue); - return result; + gunichar *ucs4 = g_utf8_to_ucs4_fast (dict->super.name, -1, NULL); + for (gunichar *it = ucs4; *it; it++) + dict->name_width += app_char_width (self, *it); + g_free (ucs4); + } + return TRUE; } -#define app_load_dictionaries app_load_dictionaries_parallel -#endif // GLib >= 2.36 - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - /// Initialize the application core. @@ -623,9 +549,8 @@ app_init (Application *self, char **filenames) self->focused = TRUE; app_init_attrs (self); - self->dictionaries = g_array_new (FALSE, TRUE, sizeof (Dictionary)); - g_array_set_clear_func - (self->dictionaries, (GDestroyNotify) dictionary_free); + self->dictionaries = + g_ptr_array_new_with_free_func ((GDestroyNotify) dictionary_destroy); GError *error = NULL; app_load_config (self, &error); @@ -640,11 +565,12 @@ app_init (Application *self, char **filenames) // Dictionaries given on the command line override the configuration if (*filenames) { - g_array_set_size (self->dictionaries, 0); + g_ptr_array_set_size (self->dictionaries, 0); while (*filenames) { - Dictionary dict = { .filename = g_strdup (*filenames++) }; - g_array_append_val (self->dictionaries, dict); + AppDictionary *dict = g_malloc0 (sizeof *dict); + dict->super.filename = g_strdup (*filenames++); + g_ptr_array_add (self->dictionaries, dict); } } @@ -659,7 +585,8 @@ app_init (Application *self, char **filenames) "the configuration or on the command line")); exit (EXIT_FAILURE); } - self->dict = g_array_index (self->dictionaries, Dictionary, 0).dict; + self->dict = ((AppDictionary *) + g_ptr_array_index (self->dictionaries, 0))->super.dict; app_reload_view (self); } @@ -715,7 +642,7 @@ app_destroy (Application *self) g_ptr_array_free (self->entries, TRUE); g_free (self->search_label); g_array_free (self->input, TRUE); - g_array_free (self->dictionaries, TRUE); + g_ptr_array_free (self->dictionaries, TRUE); g_iconv_close (self->ucs4_to_locale); } @@ -935,7 +862,7 @@ app_redraw_top (Application *self) for (guint i = 0; i < self->dictionaries->len; i++) { - Dictionary *dict = &g_array_index (self->dictionaries, Dictionary, i); + Dictionary *dict = g_ptr_array_index (self->dictionaries, i); row_buffer_append (&buf, dict->name, APP_ATTR_IF (self->dictionaries->len > 1 && self->dict == dict->dict, ACTIVE, HEADER)); @@ -1410,7 +1337,7 @@ app_goto_dictionary (Application *self, guint n) if (n >= self->dictionaries->len) return FALSE; - Dictionary *dict = &g_array_index (self->dictionaries, Dictionary, n); + Dictionary *dict = g_ptr_array_index (self->dictionaries, n); self->dict = dict->dict; app_search_for_entry (self); app_redraw_top (self); @@ -1421,13 +1348,13 @@ app_goto_dictionary (Application *self, guint n) static gboolean app_goto_dictionary_delta (Application *self, gint n) { - GArray *dicts = self->dictionaries; + GPtrArray *dicts = self->dictionaries; if (dicts->len <= 1) return FALSE; guint i = 0; while (i < dicts->len && - g_array_index (dicts, Dictionary, i).dict != self->dict) + ((Dictionary *) g_ptr_array_index (dicts, i))->dict != self->dict) i++; return app_goto_dictionary (self, (i + dicts->len + n) % dicts->len); @@ -1858,10 +1785,10 @@ app_process_left_mouse_click (Application *self, int line, int column) if (column < indent) return; - Dictionary *dicts = (Dictionary *) self->dictionaries->data; for (guint i = 0; i < self->dictionaries->len; i++) { - if (column < (indent += dicts[i].name_width)) + AppDictionary *dict = g_ptr_array_index (self->dictionaries, i); + if (column < (indent += dict->name_width)) { app_goto_dictionary (self, i); return; diff --git a/src/utils.c b/src/utils.c index 612eaaa..367c0f5 100644 --- a/src/utils.c +++ b/src/utils.c @@ -113,7 +113,7 @@ fatal (const gchar *format, ...) // At times, GLib even with its sheer size is surprisingly useless, // and I need to port some code over from "liberty". -static gchar ** +static const gchar ** get_xdg_config_dirs (void) { GPtrArray *paths = g_ptr_array_new (); @@ -122,12 +122,12 @@ get_xdg_config_dirs (void) *system; system++) g_ptr_array_add (paths, (gpointer) *system); g_ptr_array_add (paths, NULL); - return (gchar **) g_ptr_array_free (paths, FALSE); + return (const gchar **) g_ptr_array_free (paths, FALSE); } gchar * resolve_relative_filename_generic - (gchar **paths, const gchar *tail, const gchar *filename) + (const gchar **paths, const gchar *tail, const gchar *filename) { for (; *paths; paths++) { @@ -147,10 +147,10 @@ resolve_relative_filename_generic gchar * resolve_relative_config_filename (const gchar *filename) { - gchar **paths = get_xdg_config_dirs (); - gchar *result = resolve_relative_filename_generic - (paths, PROJECT_NAME, filename); - g_strfreev (paths); + const gchar **paths = get_xdg_config_dirs (); + gchar *result = + resolve_relative_filename_generic (paths, PROJECT_NAME, filename); + g_free (paths); return result; } @@ -202,7 +202,7 @@ GKeyFile * load_project_config_file (GError **error) { GKeyFile *key_file = g_key_file_new (); - gchar **paths = get_xdg_config_dirs (); + const gchar **paths = get_xdg_config_dirs (); GError *e = NULL; // XXX: if there are dashes in the final path component, @@ -210,8 +210,8 @@ load_project_config_file (GError **error) // which is completely undocumented g_key_file_load_from_dirs (key_file, PROJECT_NAME G_DIR_SEPARATOR_S PROJECT_NAME ".conf", - (const gchar **) paths, NULL, 0, &e); - g_strfreev (paths); + paths, NULL, 0, &e); + g_free (paths); if (!e) return key_file; @@ -223,3 +223,84 @@ load_project_config_file (GError **error) g_key_file_free (key_file); return NULL; } + +// --- Loading ----------------------------------------------------------------- + +void +dictionary_destroy (Dictionary *self) +{ + g_free (self->name); + g_free (self->filename); + + if (self->dict) + g_object_unref (self->dict); + + g_free (self); +} + +static gboolean +dictionary_load (Dictionary *self, GError **e) +{ + if (!(self->dict = stardict_dict_new (self->filename, e))) + return FALSE; + + if (!self->name) + { + self->name = g_strdup (stardict_info_get_book_name + (stardict_dict_get_info (self->dict))); + } + return TRUE; +} + +static gboolean +load_dictionaries_sequentially (GPtrArray *dictionaries, GError **e) +{ + for (guint i = 0; i < dictionaries->len; i++) + if (!dictionary_load (g_ptr_array_index (dictionaries, i), e)) + return FALSE; + return TRUE; +} + +// Parallelize dictionary loading if possible, because of collation reindexing +#if GLIB_CHECK_VERSION (2, 36, 0) +static void +load_worker (gpointer data, gpointer user_data) +{ + GError *e = NULL; + dictionary_load (data, &e); + if (e) + g_async_queue_push (user_data, e); +} + +gboolean +load_dictionaries (GPtrArray *dictionaries, GError **e) +{ + GAsyncQueue *error_queue = + g_async_queue_new_full ((GDestroyNotify) g_error_free); + GThreadPool *pool = g_thread_pool_new (load_worker, error_queue, + g_get_num_processors (), TRUE, NULL); + if G_UNLIKELY (!g_thread_pool_get_num_threads (pool)) + { + g_thread_pool_free (pool, TRUE, TRUE); + g_async_queue_unref (error_queue); + return load_dictionaries_sequentially (dictionaries, e); + } + + for (guint i = 0; i < dictionaries->len; i++) + g_thread_pool_push (pool, g_ptr_array_index (dictionaries, i), NULL); + g_thread_pool_free (pool, FALSE, TRUE); + + gboolean result = TRUE; + if ((*e = g_async_queue_try_pop (error_queue))) + result = FALSE; + + g_async_queue_unref (error_queue); + return result; +} +#else // GLib < 2.36 +gboolean +load_dictionaries (GPtrArray *dictionaries, GError **e) +{ + return load_dictionaries_sequentially (dictionaries, e); +} +#endif // GLib < 2.36 diff --git a/src/utils.h b/src/utils.h index 1394f08..bc5775b 100644 --- a/src/utils.h +++ b/src/utils.h @@ -19,6 +19,11 @@ #ifndef UTILS_H #define UTILS_H +#include +#include + +#include "stardict.h" + /// After this statement, the element has been found and its index is stored /// in the variable "imid". #define BINARY_SEARCH_BEGIN(max, compare) \ @@ -44,10 +49,24 @@ gboolean xstrtoul (unsigned long *out, const char *s, int base); void fatal (const gchar *format, ...) G_GNUC_PRINTF (1, 2) G_GNUC_NORETURN; gchar *resolve_relative_filename_generic - (gchar **paths, const gchar *tail, const gchar *filename); + (const gchar **paths, const gchar *tail, const gchar *filename); gchar *resolve_relative_config_filename (const gchar *filename); gchar *resolve_filename (const gchar *filename, gchar *(*relative_cb) (const char *)); GKeyFile *load_project_config_file (GError **error); +// --- Loading ----------------------------------------------------------------- + +typedef struct dictionary Dictionary; + +struct dictionary +{ + gchar *filename; ///< Path to the dictionary + StardictDict *dict; ///< StarDict dictionary data + gchar *name; ///< Name to show +}; + +void dictionary_destroy (Dictionary *self); +gboolean load_dictionaries (GPtrArray *dictionaries, GError **e); + #endif // ! UTILS_H -- cgit v1.2.3-70-g09d2