Mercurial > hg > forks > geeqie
changeset 2240:d3a300d826d0
Merge remote-tracking branches 'merge-requests/6' and 'merge-requests/7'
* merge-requests/6:
Bug 3594998: make lirc initialization quieter
* merge-requests/7:
Add filedata counting to watch for filedata leaks
Use FileData locks to avoid expensive reloads with marks enabled
Add "lock" functionality to keep FileDatas in memory
author | Klaus Ethgen <Klaus@Ethgen.de> |
---|---|
date | Sun, 20 Jul 2014 13:57:40 +0100 |
parents | 87ed572f1eea (current diff) 7d1fe1247eda (diff) |
children | 4c30cb6fcf93 |
files | src/filedata.c |
diffstat | 5 files changed, 207 insertions(+), 40 deletions(-) [+] |
line wrap: on
line diff
--- a/src/filedata.c Sun Jul 20 13:54:56 2014 +0100 +++ b/src/filedata.c Sun Jul 20 13:57:40 2014 +0100 @@ -26,6 +26,10 @@ #include <errno.h> +#ifdef DEBUG_FILEDATA +gint global_file_data_count = 0; +#endif + static GHashTable *file_data_pool = NULL; static GHashTable *file_data_planned_change_hash = NULL; @@ -383,6 +387,10 @@ } fd = g_new0(FileData, 1); +#ifdef DEBUG_FILEDATA + global_file_data_count++; + DEBUG_2("file data count++: %d", global_file_data_count); +#endif fd->size = st->st_size; fd->date = st->st_mtime; @@ -532,6 +540,12 @@ { g_assert(fd->magick == FD_MAGICK); g_assert(fd->ref == 0); + g_assert(!fd->locked); + +#ifdef DEBUG_FILEDATA + global_file_data_count--; + DEBUG_2("file data count--: %d", global_file_data_count); +#endif metadata_cache_free(fd); g_hash_table_remove(file_data_pool, fd->original_path); @@ -549,6 +563,57 @@ g_free(fd); } +/** + * \brief Checks if the FileData is referenced + * + * Checks the refcount and whether the FileData is locked. + */ +static gboolean file_data_check_has_ref(FileData *fd) +{ + return fd->ref > 0 || fd->locked; +} + +/** + * \brief Consider freeing a FileData. + * + * This function will free a FileData and its children provided that neither its parent nor it has + * a positive refcount, and provided that neither is locked. + */ +static void file_data_consider_free(FileData *fd) +{ + GList *work; + FileData *parent = fd->parent ? fd->parent : fd; + + g_assert(fd->magick == FD_MAGICK); + if (file_data_check_has_ref(fd)) return; + if (file_data_check_has_ref(parent)) return; + + work = parent->sidecar_files; + while (work) + { + FileData *sfd = work->data; + if (file_data_check_has_ref(sfd)) return; + work = work->next; + } + + /* Neither the parent nor the siblings are referenced, so we can free everything */ + DEBUG_2("file_data_consider_free: deleting '%s', parent '%s'", + fd->path, fd->parent ? parent->path : "-"); + + work = parent->sidecar_files; + while (work) + { + FileData *sfd = work->data; + file_data_free(sfd); + work = work->next; + } + + g_list_free(parent->sidecar_files); + parent->sidecar_files = NULL; + + file_data_free(parent); +} + #ifdef DEBUG_FILEDATA void file_data_unref_debug(const gchar *file, gint line, FileData *fd) #else @@ -566,45 +631,92 @@ fd->ref--; #ifdef DEBUG_FILEDATA - DEBUG_2("file_data_unref fd=%p (%d): '%s' @ %s:%d", fd, fd->ref, fd->path, file, line); + DEBUG_2("file_data_unref fd=%p (%d:%d): '%s' @ %s:%d", fd, fd->ref, fd->locked, fd->path, + file, line); #else - DEBUG_2("file_data_unref fd=%p (%d): '%s'", fd, fd->ref, fd->path); + DEBUG_2("file_data_unref fd=%p (%d:%d): '%s'", fd, fd->ref, fd->locked, fd->path); #endif - if (fd->ref == 0) + + // Free FileData if it's no longer ref'd + file_data_consider_free(fd); +} + +/** + * \brief Lock the FileData in memory. + * + * This allows the caller to prevent a FileData from being freed, even after its refcount is zero. + * This is intended to be used in cases where a FileData _should_ stay in memory as an optimization, + * even if the code would continue to function properly even if the FileData were freed. Code that + * _requires_ the FileData to remain in memory should continue to use file_data_(un)ref. + * <p /> + * Note: This differs from file_data_ref in that the behavior is reentrant -- after N calls to + * file_data_lock, a single call to file_data_unlock will unlock the FileData. + */ +void file_data_lock(FileData *fd) +{ + if (fd == NULL) return; + if (fd->magick != FD_MAGICK) DEBUG_0("fd magick mismatch fd=%p", fd); + + g_assert(fd->magick == FD_MAGICK); + fd->locked = TRUE; + + DEBUG_2("file_data_ref fd=%p (%d): '%s'", fd, fd->ref, fd->path); +} + +/** + * \brief Reset the maintain-FileData-in-memory lock + * + * This again allows the FileData to be freed when its refcount drops to zero. Automatically frees + * the FileData if its refcount is already zero (which will happen if the lock is the only thing + * keeping it from being freed. + */ +void file_data_unlock(FileData *fd) +{ + if (fd == NULL) return; + if (fd->magick != FD_MAGICK) DEBUG_0("fd magick mismatch fd=%p", fd); + + g_assert(fd->magick == FD_MAGICK); + fd->locked = FALSE; + + // Free FileData if it's no longer ref'd + file_data_consider_free(fd); +} + +/** + * \brief Lock all of the FileDatas in the provided list + * + * \see file_data_lock(FileData) + */ +void file_data_lock_list(GList *list) +{ + GList *work; + + work = list; + while (work) { - GList *work; - FileData *parent = fd->parent ? fd->parent : fd; - - if (parent->ref > 0) return; - - work = parent->sidecar_files; - while (work) - { - FileData *sfd = work->data; - if (sfd->ref > 0) return; - work = work->next; - } - - /* none of parent/children is referenced, we can free everything */ - - DEBUG_2("file_data_unref: deleting '%s', parent '%s'", fd->path, fd->parent ? parent->path : "-"); - - work = parent->sidecar_files; - while (work) - { - FileData *sfd = work->data; - file_data_free(sfd); - work = work->next; - } - - g_list_free(parent->sidecar_files); - parent->sidecar_files = NULL; - - file_data_free(parent); + FileData *fd = work->data; + work = work->next; + file_data_lock(fd); } } - +/** + * \brief Unlock all of the FileDatas in the provided list + * + * \see file_data_unlock(FileData) + */ +void file_data_unlock_list(GList *list) +{ + GList *work; + + work = list; + while (work) + { + FileData *fd = work->data; + work = work->next; + file_data_unlock(fd); + } +} /* *-----------------------------------------------------------------------------
--- a/src/filedata.h Sun Jul 20 13:54:56 2014 +0100 +++ b/src/filedata.h Sun Jul 20 13:57:40 2014 +0100 @@ -43,6 +43,11 @@ void file_data_unref(FileData *fd); #endif +void file_data_lock(FileData *fd); +void file_data_unlock(FileData *fd); +void file_data_lock_list(GList *list); +void file_data_unlock_list(GList *list); + gboolean file_data_check_changed_files(FileData *fd); void file_data_increment_version(FileData *fd);
--- a/src/lirc.c Sun Jul 20 13:54:56 2014 +0100 +++ b/src/lirc.c Sun Jul 20 13:57:40 2014 +0100 @@ -204,30 +204,37 @@ void layout_image_lirc_init(LayoutWindow *lw) { gint flags; + gboolean lirc_verbose = (get_debug_level() >= 2); - DEBUG_1("Initializing LIRC..."); - lirc_fd = lirc_init(GQ_APPNAME_LC, get_debug_level() > 0); + lirc_fd = lirc_init(GQ_APPNAME_LC, lirc_verbose); if (lirc_fd == -1) { - g_fprintf(stderr, _("Could not init LIRC support\n")); + DEBUG_1("Initializing LIRC... failed"); return; } + + DEBUG_1("Initializing LIRC... OK"); if (lirc_readconfig(NULL, &config, NULL) == -1) { lirc_deinit(); + g_fprintf(stderr, _("could not read LIRC config file\n" "please read the documentation of LIRC to \n" "know how to create a proper config file\n")); + fflush(stderr); + + DEBUG_1("Failed to read LIRC config file"); return; } + if (lirc_verbose) fflush(stderr); + gio_chan = g_io_channel_unix_new(lirc_fd); input_tag = g_io_add_watch(gio_chan, G_IO_IN, lirc_input_callback, lw); fcntl(lirc_fd, F_SETOWN, getpid()); flags = fcntl(lirc_fd, F_GETFL, 0); if (flags != -1) fcntl(lirc_fd, F_SETFL, flags|O_NONBLOCK); - fflush(stderr); } #endif /* HAVE_LIRC */
--- a/src/typedefs.h Sun Jul 20 13:54:56 2014 +0100 +++ b/src/typedefs.h Sun Jul 20 13:57:40 2014 +0100 @@ -514,6 +514,7 @@ HistMap *histmap; + gboolean locked; gint ref; gint version; /* increased when any field in this structure is changed */ gboolean disable_grouping;
--- a/src/view_file_list.c Sun Jul 20 13:54:56 2014 +0100 +++ b/src/view_file_list.c Sun Jul 20 13:57:40 2014 +0100 @@ -148,9 +148,25 @@ return FALSE; } -static void vflist_store_clear(ViewFile *vf) +static void vflist_store_clear(ViewFile *vf, gboolean unlock_files) { GtkTreeModel *store; + GList *files = NULL; + + if (unlock_files && vf->marks_enabled) + { + // unlock locked files in this directory + filelist_read(vf->dir_fd, &files, NULL); + while (files) + { + FileData *fd = files->data; + files = files->next; + file_data_unlock(fd); + file_data_unref(fd); // undo the ref that got added in filelist_read + } + } + + g_list_free(files); store = gtk_tree_view_get_model(GTK_TREE_VIEW(vf->listview)); gtk_tree_model_foreach(store, vflist_store_clear_cb, NULL); gtk_tree_store_clear(GTK_TREE_STORE(store)); @@ -1649,7 +1665,7 @@ if (!vf->list) { - vflist_store_clear(vf); + vflist_store_clear(vf, FALSE); vf_send_update(vf); return; } @@ -1686,6 +1702,20 @@ file_data_unregister_notify_func(vf_notify_cb, vf); /* we don't need the notification of changes detected by filelist_read */ ret = filelist_read(vf->dir_fd, &vf->list, NULL); + + if (vf->marks_enabled) + { + // When marks are enabled, lock FileDatas so that we don't end up re-parsing XML + // each time a mark is changed. + file_data_lock_list(vf->list); + } + else + { + // FIXME: only do this when needed (aka when we just switched from + // FIXME: marks-enabled to marks-disabled) + file_data_unlock_list(vf->list); + } + vf->list = file_data_filter_marks_list(vf->list, vf_marks_get_filter(vf)); file_data_register_notify_func(vf_notify_cb, vf, NOTIFY_PRIORITY_MEDIUM); @@ -1697,6 +1727,8 @@ vflist_populate_view(vf, FALSE); + DEBUG_1("%s vflist_refresh: free filelist", get_exec_time()); + filelist_free(old_list); DEBUG_1("%s vflist_refresh: done", get_exec_time()); @@ -1868,7 +1900,7 @@ vf->dir_fd = file_data_ref(dir_fd); /* force complete reload */ - vflist_store_clear(vf); + vflist_store_clear(vf, TRUE); filelist_free(vf->list); vf->list = NULL; @@ -1995,6 +2027,16 @@ gtk_tree_view_column_set_visible(column, enable); } + if (enable) + { + // Previously disabled, which means that vf->list is complete + file_data_lock_list(vf->list); + } + else + { + // Previously enabled, which means that vf->list is incomplete + } + g_list_free(columns); }