Use FileData locks to avoid expensive reloads with marks enabled

diff --git a/src/filedata.c b/src/filedata.c
index bc9685c..6adafc2 100644 (file)
--- a/src/filedata.c
+++ b/src/filedata.c
@@ -532,6 +532,7 @@ static void file_data_free(FileData *fd)
 {
        g_assert(fd->magick == FD_MAGICK);
        g_assert(fd->ref == 0);
 {
        g_assert(fd->magick == FD_MAGICK);
        g_assert(fd->ref == 0);

+       g_assert(!fd->locked);

 
        metadata_cache_free(fd);
        g_hash_table_remove(file_data_pool, fd->original_path);
 
        metadata_cache_free(fd);
        g_hash_table_remove(file_data_pool, fd->original_path);


@@ -630,10 +631,12 @@ void file_data_unref(FileData *fd)
 /**
  * \brief Lock the FileData in memory.
  *
 /**
  * \brief Lock the FileData in memory.
  *

- * This allows the caller to prevent a FileData from being freed, even
- * after its refcount is zero.
+ * 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 />
  * <p />

- * This differs from file_data_ref in that the behavior is reentrant -- after N calls to
+ * 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)
  * file_data_lock, a single call to file_data_unlock will unlock the FileData.
  */
 void file_data_lock(FileData *fd)


@@ -666,6 +669,42 @@ void file_data_unlock(FileData *fd)
        file_data_consider_free(fd);
 }
 
        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)
+               {
+               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);
+               }
+}
+

 /*
  *-----------------------------------------------------------------------------
  * sidecar file info struct
 /*
  *-----------------------------------------------------------------------------
  * sidecar file info struct

diff --git a/src/filedata.h b/src/filedata.h
index c7e9e89..43edc42 100644 (file)
--- a/src/filedata.h
+++ b/src/filedata.h
@@ -45,6 +45,8 @@ void file_data_unref(FileData *fd);
 
 void file_data_lock(FileData *fd);
 void file_data_unlock(FileData *fd);
 
 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);
 
 
 gboolean file_data_check_changed_files(FileData *fd);
 

diff --git a/src/view_file_list.c b/src/view_file_list.c
index c8a1cd6..8d2d80c 100644 (file)
--- a/src/view_file_list.c
+++ b/src/view_file_list.c
@@ -148,9 +148,25 @@ static gboolean vflist_store_clear_cb(GtkTreeModel *model, GtkTreePath *path, Gt
        return FALSE;
 }
 
        return FALSE;
 }
 

-static void vflist_store_clear(ViewFile *vf)
+static void vflist_store_clear(ViewFile *vf, gboolean unlock_files)

 {
        GtkTreeModel *store;
 {
        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));
        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 @@ static void vflist_populate_view(ViewFile *vf, gboolean force)
 
        if (!vf->list)
                {
 
        if (!vf->list)
                {

-               vflist_store_clear(vf);
+               vflist_store_clear(vf, FALSE);

                vf_send_update(vf);
                return;
                }
                vf_send_update(vf);
                return;
                }


@@ -1686,6 +1702,20 @@ gboolean vflist_refresh(ViewFile *vf)
                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);
                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);
 
                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 @@ gboolean vflist_refresh(ViewFile *vf)
 
        vflist_populate_view(vf, FALSE);
 
 
        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());
 
        filelist_free(old_list);
        DEBUG_1("%s vflist_refresh: done", get_exec_time());
 


@@ -1868,7 +1900,7 @@ gboolean vflist_set_fd(ViewFile *vf, FileData *dir_fd)
        vf->dir_fd = file_data_ref(dir_fd);
 
        /* force complete reload */
        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;
 
        filelist_free(vf->list);
        vf->list = NULL;


@@ -1995,6 +2027,16 @@ void vflist_marks_set(ViewFile *vf, gboolean enable)
                        gtk_tree_view_column_set_visible(column, enable);
                }
 
                        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);
 }
 
        g_list_free(columns);
 }