Cache the number of days read only value

The KVP value for the qof_book_get_num_days_autoreadonly was being
called many times so it makes sense to cache it in the book to avoid
the KVP lookup.

gnucash/gnome-utils/gnc-main-window.c

@@ -4025,7 +4025,8 @@ gnc_book_options_dialog_apply_helper(GNCOptionDB * options)
         qof_book_use_split_action_for_num_field (book);
     gboolean use_book_currency_before =
         gnc_book_use_book_currency (book);
-    gint use_read_only_threshold_before = qof_book_get_num_days_autoreadonly (book);
+    gint use_read_only_threshold_before =
+        qof_book_get_num_days_autoreadonly (book);
     gboolean use_split_action_for_num_after;
     gboolean use_book_currency_after;
     gint use_read_only_threshold_after;
@@ -4037,7 +4038,7 @@ gnc_book_options_dialog_apply_helper(GNCOptionDB * options)
     results = gnc_option_db_commit (options);
     for (iter = results; iter; iter = iter->next)
     {
-        GtkWidget *dialog = gtk_message_dialog_new(NULL,
+        GtkWidget *dialog = gtk_message_dialog_new(gnc_ui_get_main_window (NULL),
                                                    0,
                                                    GTK_MESSAGE_ERROR,
                                                    GTK_BUTTONS_OK,
@@ -4053,7 +4054,11 @@ gnc_book_options_dialog_apply_helper(GNCOptionDB * options)
     use_split_action_for_num_after =
         qof_book_use_split_action_for_num_field (book);
     use_book_currency_after = gnc_book_use_book_currency (book);
+
+    // mark cached value as invalid so we get new value
+    book->cached_num_days_autoreadonly_isvalid = FALSE;
     use_read_only_threshold_after = qof_book_get_num_days_autoreadonly (book);
+
     if (use_split_action_for_num_before != use_split_action_for_num_after)
     {
         gnc_book_option_num_field_source_change_cb (

libgnucash/engine/qofbook.cpp

@@ -96,8 +96,14 @@ static void
 qof_book_option_num_field_source_changed_cb (GObject *gobject,
                                              GParamSpec *pspec,
                                              gpointer    user_data);
+static void
+qof_book_option_num_autoreadonly_changed_cb (GObject *gobject,
+                                             GParamSpec *pspec,
+                                             gpointer    user_data);
+
 // Use a #define for the GParam name to avoid typos
 #define PARAM_NAME_NUM_FIELD_SOURCE "split-action-num-field"
+#define PARAM_NAME_NUM_AUTOREAD_ONLY "autoreadonly-days"
 
 QOF_GOBJECT_GET_TYPE(QofBook, qof_book, QOF_TYPE_INSTANCE, {});
 QOF_GOBJECT_DISPOSE(qof_book);
@@ -134,6 +140,7 @@ qof_book_init (QofBook *book)
     book->session_dirty = FALSE;
     book->version = 0;
     book->cached_num_field_source_isvalid = FALSE;
+    book->cached_num_days_autoreadonly_isvalid = FALSE;
 
     // Register a callback on this NUM_FIELD_SOURCE property of that object
     // because it gets called quite a lot, so that its value must be stored in
@@ -142,6 +149,14 @@ qof_book_init (QofBook *book)
                       "notify::" PARAM_NAME_NUM_FIELD_SOURCE,
                       G_CALLBACK (qof_book_option_num_field_source_changed_cb),
                       book);
+
+    // Register a callback on this NUM_AUTOREAD_ONLY property of that object
+    // because it gets called quite a lot, so that its value must be stored in
+    // a bool member variable instead of a KVP lookup on each getter call.
+    g_signal_connect (G_OBJECT(book),
+                      "notify::" PARAM_NAME_NUM_AUTOREAD_ONLY,
+                      G_CALLBACK (qof_book_option_num_autoreadonly_changed_cb),
+                      book);
 }
 
 static const std::string str_KVP_OPTION_PATH(KVP_OPTION_PATH);
@@ -1065,11 +1080,21 @@ gboolean qof_book_uses_autoreadonly (const QofBook *book)
 gint qof_book_get_num_days_autoreadonly (const QofBook *book)
 {
     g_assert(book);
-    double tmp;
-    qof_instance_get (QOF_INSTANCE (book),
-		      "autoreadonly-days", &tmp,
-		      NULL);
-    return (gint) tmp;
+
+    if (!book->cached_num_days_autoreadonly_isvalid)
+    {
+        double tmp;
+
+        // No cached value? Then do the expensive KVP lookup
+        qof_instance_get (QOF_INSTANCE (book),
+              PARAM_NAME_NUM_AUTOREAD_ONLY, &tmp,
+              NULL);
+
+        const_cast<QofBook*>(book)->cached_num_days_autoreadonly = tmp;
+        const_cast<QofBook*>(book)->cached_num_days_autoreadonly_isvalid = TRUE;
+    }
+    // Value is cached now. Use the cheap variable returning.
+    return (gint) book->cached_num_days_autoreadonly;
 }
 
 GDate* qof_book_get_autoreadonly_gdate (const QofBook *book)
@@ -1087,6 +1112,19 @@ GDate* qof_book_get_autoreadonly_gdate (const QofBook *book)
     return result;
 }
 
+// The callback that is called when the KVP option value of
+// "autoreadonly-days" changes, so that we mark the cached value as
+// invalid.
+static void
+qof_book_option_num_autoreadonly_changed_cb (GObject *gobject,
+                                             GParamSpec *pspec,
+                                             gpointer    user_data)
+{
+    QofBook *book = reinterpret_cast<QofBook*>(user_data);
+    g_return_if_fail(QOF_IS_BOOK(book));
+    book->cached_num_days_autoreadonly_isvalid = FALSE;
+}
+
 /* Note: this will fail if the book slots we're looking for here are flattened at some point !
  * When that happens, this function can be removed. */
 static Path opt_name_to_path (const char* opt_name)

libgnucash/engine/qofbook.h

@@ -154,6 +154,12 @@ struct _QofBook
     gboolean cached_num_field_source;
     /* Whether the above cached value is valid. */
     gboolean cached_num_field_source_isvalid;
+
+    /* A cahed value of the "autoreadonly-days" option value because it is
+     * queried quite a lot, so we want to avoid a KVP lookup on each query */
+    gint cached_num_days_autoreadonly;
+    /* Whether the above cached value is valid. */
+    gboolean cached_num_days_autoreadonly_isvalid;
 };
 
 struct _QofBookClass