[epiphany/gnome-3-22] history-service: remove longstanding transactions

[Michael Catanzaro <mcatanzaro src gnome org>]

commit cfe3911b43de2ebf3c77e3377639a0bc51f7a12b
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Tue Feb 21 10:36:18 2017 -0600

    history-service: remove longstanding transactions
    
    Instead of having a longstanding transaction open at all times and
    scheduling commits that open a new transaction, just create transactions
    around history messages, so that each message forms its own atomic
    transaction. This is way simpler.
    
    I considered that we might not need transactions at all, but there are
    performance implications to removing transactions entirely.

 lib/history/ephy-history-service-hosts-table.c  |    1 -
 lib/history/ephy-history-service-private.h      |    2 -
 lib/history/ephy-history-service-urls-table.c   |    1 -
 lib/history/ephy-history-service-visits-table.c |    7 +-
 lib/history/ephy-history-service.c              |  135 ++++++++---------------
 5 files changed, 47 insertions(+), 99 deletions(-)
---
diff --git a/lib/history/ephy-history-service-hosts-table.c b/lib/history/ephy-history-service-hosts-table.c
index a4f1abb..472efd1 100644
--- a/lib/history/ephy-history-service-hosts-table.c
+++ b/lib/history/ephy-history-service-hosts-table.c
@@ -45,7 +45,6 @@ ephy_history_service_initialize_hosts_table (EphyHistoryService *self)
     g_error_free (error);
     return FALSE;
   }
-  ephy_history_service_schedule_commit (self);
   return TRUE;
 }
 
diff --git a/lib/history/ephy-history-service-private.h b/lib/history/ephy-history-service-private.h
index 51b4705..5443180 100644
--- a/lib/history/ephy-history-service-private.h
+++ b/lib/history/ephy-history-service-private.h
@@ -31,12 +31,10 @@ struct _EphyHistoryService {
   GThread *history_thread;
   GAsyncQueue *queue;
   gboolean scheduled_to_quit;
-  gboolean scheduled_to_commit;
   gboolean read_only;
   int queue_urls_visited_id;
 };
 
-void                     ephy_history_service_schedule_commit         (EphyHistoryService *self); 
 gboolean                 ephy_history_service_initialize_urls_table   (EphyHistoryService *self);
 EphyHistoryURL *         ephy_history_service_get_url_row             (EphyHistoryService *self, const char 
*url_string, EphyHistoryURL *url);
 void                     ephy_history_service_add_url_row             (EphyHistoryService *self, 
EphyHistoryURL *url);
diff --git a/lib/history/ephy-history-service-urls-table.c b/lib/history/ephy-history-service-urls-table.c
index 602d073..7a71522 100644
--- a/lib/history/ephy-history-service-urls-table.c
+++ b/lib/history/ephy-history-service-urls-table.c
@@ -47,7 +47,6 @@ ephy_history_service_initialize_urls_table (EphyHistoryService *self)
     g_error_free (error);
     return FALSE;
   }
-  ephy_history_service_schedule_commit (self);
   return TRUE;
 }
 
diff --git a/lib/history/ephy-history-service-visits-table.c b/lib/history/ephy-history-service-visits-table.c
index 855d6c2..56b1d11 100644
--- a/lib/history/ephy-history-service-visits-table.c
+++ b/lib/history/ephy-history-service-visits-table.c
@@ -41,7 +41,7 @@ ephy_history_service_initialize_visits_table (EphyHistoryService *self)
     g_error_free (error);
     return FALSE;
   }
-  ephy_history_service_schedule_commit (self);
+
   return TRUE;
 }
 
@@ -81,12 +81,7 @@ ephy_history_service_add_visit_row (EphyHistoryService *self, EphyHistoryPageVis
     visit->id = ephy_sqlite_connection_get_last_insert_id (self->history_database);
   }
 
-  /* Remember kids: prepared statements lock the database! They must be unreffed
-   * before scheduling commit.
-   */
   g_object_unref (statement);
-
-  ephy_history_service_schedule_commit (self);
 }
 
 static EphyHistoryPageVisit *
diff --git a/lib/history/ephy-history-service.c b/lib/history/ephy-history-service.c
index 3e15749..0f12e6e 100644
--- a/lib/history/ephy-history-service.c
+++ b/lib/history/ephy-history-service.c
@@ -355,29 +355,37 @@ ephy_history_service_send_message (EphyHistoryService *self, EphyHistoryServiceM
 }
 
 static void
-ephy_history_service_commit (EphyHistoryService *self)
+ephy_history_service_open_transaction (EphyHistoryService *self)
 {
   GError *error = NULL;
   g_assert (self->history_thread == g_thread_self ());
 
-  if (self->history_database == NULL)
-    return;
-
-  if (self->read_only)
+  if (self->history_database == NULL ||
+      self->read_only)
     return;
 
-  ephy_sqlite_connection_commit_transaction (self->history_database, &error);
+  ephy_sqlite_connection_begin_transaction (self->history_database, &error);
   if (error != NULL) {
-    g_warning ("Could not commit idle history database transaction: %s", error->message);
+    g_error ("Could not open history database transaction: %s", error->message);
     g_error_free (error);
   }
-  ephy_sqlite_connection_begin_transaction (self->history_database, &error);
+}
+
+static void
+ephy_history_service_commit_transaction (EphyHistoryService *self)
+{
+  GError *error = NULL;
+  g_assert (self->history_thread == g_thread_self ());
+
+  if (self->history_database == NULL ||
+      self->read_only)
+    return;
+
+  ephy_sqlite_connection_commit_transaction (self->history_database, &error);
   if (error != NULL) {
-    g_warning ("Could not start long-running history database transaction: %s", error->message);
+    g_warning ("Could not commit history database transaction: %s", error->message);
     g_error_free (error);
   }
-
-  self->scheduled_to_commit = FALSE;
 }
 
 static void
@@ -427,22 +435,10 @@ ephy_history_service_open_database_connections (EphyHistoryService *self)
 
   ephy_history_service_enable_foreign_keys (self);
 
-  if (self->read_only)
-    return TRUE;
-
-  ephy_sqlite_connection_begin_transaction (self->history_database, &error);
-  if (error) {
-    g_warning ("Could not begin long running transaction in history database: %s", error->message);
-    g_error_free (error);
-    return FALSE;
-  }
-
-  if (ephy_history_service_initialize_hosts_table (self) == FALSE ||
-      ephy_history_service_initialize_urls_table (self) == FALSE ||
-      ephy_history_service_initialize_visits_table (self) == FALSE)
-    return FALSE;
-
-  return TRUE;
+  return self->read_only ||
+          (ephy_history_service_initialize_hosts_table (self) &&
+           ephy_history_service_initialize_urls_table (self) &&
+           ephy_history_service_initialize_visits_table (self));
 }
 
 static void
@@ -455,57 +451,11 @@ ephy_history_service_close_database_connections (EphyHistoryService *self)
   self->history_database = NULL;
 }
 
-static void
-ephy_history_service_clear_all (EphyHistoryService *self)
-{
-  char *journal_filename;
-
-  if (self->history_database == NULL)
-    return;
-
-  if (self->read_only)
-    return;
-
-  ephy_sqlite_connection_close (self->history_database);
-
-  if (g_unlink (self->history_filename) == -1)
-    g_warning ("Failed to delete %s: %s", self->history_filename, g_strerror (errno));
-
-  journal_filename = g_strdup_printf ("%s-journal", self->history_filename);
-  if (g_unlink (journal_filename) == -1 && errno != ENOENT)
-    g_warning ("Failed to delete %s: %s", journal_filename, g_strerror (errno));
-  g_free (journal_filename);
-
-  ephy_history_service_open_database_connections (self);
-}
-
-static gboolean
-ephy_history_service_is_scheduled_to_quit (EphyHistoryService *self)
-{
-  return self->scheduled_to_quit;
-}
-
-static gboolean
-ephy_history_service_is_scheduled_to_commit (EphyHistoryService *self)
-{
-  return self->scheduled_to_commit;
-}
-
-void
-ephy_history_service_schedule_commit (EphyHistoryService *self)
-{
-  if (!self->read_only)
-    self->scheduled_to_commit = TRUE;
-}
-
 static gboolean
 ephy_history_service_execute_quit (EphyHistoryService *self, gpointer data, gpointer *result)
 {
   g_assert (self->history_thread == g_thread_self ());
 
-  if (ephy_history_service_is_scheduled_to_commit (self))
-    ephy_history_service_commit (self);
-
   g_async_queue_unref (self->queue);
 
   self->scheduled_to_quit = TRUE;
@@ -539,16 +489,13 @@ run_history_service_thread (EphyHistoryService *self)
   do {
     message = g_async_queue_try_pop (self->queue);
     if (!message) {
-      if (ephy_history_service_is_scheduled_to_commit (self))
-        ephy_history_service_commit (self);
-
       /* Block the thread until there's data in the queue. */
       message = g_async_queue_pop (self->queue);
     }
 
     /* Process item. */
     ephy_history_service_process_message (self, message);
-  } while (!ephy_history_service_is_scheduled_to_quit (self));
+  } while (!self->scheduled_to_quit);
 
   ephy_history_service_close_database_connections (self);
 
@@ -676,8 +623,6 @@ ephy_history_service_execute_add_visits (EphyHistoryService *self, GList *visits
     visits = visits->next;
   }
 
-  ephy_history_service_schedule_commit (self);
-
   return success;
 }
 
@@ -873,7 +818,6 @@ ephy_history_service_execute_set_url_title (EphyHistoryService *self,
     g_free (url->title);
     url->title = title;
     ephy_history_service_update_url_row (self, url);
-    ephy_history_service_schedule_commit (self);
 
     ctx = signal_emission_context_new (self,
                                        ephy_history_url_copy (url),
@@ -929,7 +873,6 @@ ephy_history_service_execute_set_url_zoom_level (EphyHistoryService *self,
 
   host->zoom_level = zoom_level;
   ephy_history_service_update_host_row (self, host);
-  ephy_history_service_schedule_commit (self);
 
   return TRUE;
 }
@@ -974,7 +917,6 @@ ephy_history_service_execute_set_url_hidden (EphyHistoryService *self,
   } else {
     url->hidden = hidden;
     ephy_history_service_update_url_row (self, url);
-    ephy_history_service_schedule_commit (self);
     return TRUE;
   }
 }
@@ -1019,7 +961,6 @@ ephy_history_service_execute_set_url_thumbnail_time (EphyHistoryService *self,
   else {
     url->thumbnail_time = thumbnail_time;
     ephy_history_service_update_url_row (self, url);
-    ephy_history_service_schedule_commit (self);
     return TRUE;
   }
 }
@@ -1146,7 +1087,6 @@ ephy_history_service_execute_delete_urls (EphyHistoryService *self,
   }
 
   ephy_history_service_delete_orphan_hosts (self);
-  ephy_history_service_schedule_commit (self);
 
   return TRUE;
 }
@@ -1173,7 +1113,6 @@ ephy_history_service_execute_delete_host (EphyHistoryService    *self,
     return FALSE;
 
   ephy_history_service_delete_host_row (self, host);
-  ephy_history_service_schedule_commit (self);
 
   ctx = signal_emission_context_new (self, g_strdup (host->url),
                                      (GDestroyNotify)g_free);
@@ -1190,10 +1129,25 @@ ephy_history_service_execute_clear (EphyHistoryService *self,
                                     gpointer            pointer,
                                     gpointer           *result)
 {
-  if (self->read_only)
+  char *journal_filename;
+
+  if (self->history_database == NULL ||
+      self->read_only)
     return FALSE;
 
-  ephy_history_service_clear_all (self);
+  ephy_history_service_commit_transaction (self);
+  ephy_sqlite_connection_close (self->history_database);
+
+  if (g_unlink (self->history_filename) == -1)
+    g_warning ("Failed to delete %s: %s", self->history_filename, g_strerror (errno));
+
+  journal_filename = g_strdup_printf ("%s-journal", self->history_filename);
+  if (g_unlink (journal_filename) == -1 && errno != ENOENT)
+    g_warning ("Failed to delete %s: %s", journal_filename, g_strerror (errno));
+  g_free (journal_filename);
+
+  ephy_history_service_open_database_connections (self);
+  ephy_history_service_open_transaction (self);
 
   return TRUE;
 }
@@ -1299,10 +1253,13 @@ ephy_history_service_process_message (EphyHistoryService        *self,
 
   method = methods[message->type];
   message->result = NULL;
-  if (message->service->history_database)
+  if (message->service->history_database) {
+    ephy_history_service_open_transaction (self);
     message->success = method (message->service, message->method_argument, &message->result);
-  else
+    ephy_history_service_commit_transaction (self);
+  } else {
     message->success = FALSE;
+  }
 
   if (message->callback || message->type == CLEAR)
     g_idle_add ((GSourceFunc)ephy_history_service_execute_job_callback, message);