Register speed-up for large files.

The function qof_book_use_split_action_for_num_field gets called quite a
lot in each register display refresh (due to sorting all splits from
Split.x's xaccSplitOrder function), but it always used to use a KVP
lookup, which is rather expensive compared to accessing a gboolean member
variable.

To get rid of this cost, I had to remove the KVP lookup in this
simple-looking function. The pattern is this: A gboolean cache variable is
introduced, along with an isvalid flag. The lookup makes the expensive
KVP lookup once, then caches the value. The GObject property mechanism
offers a callback for when the setter was called, which is used to mark
the cached value as invalid. A parallel setter method (here:
qof_book_set_option) also just marks the cache as invalid. This covers
all setters, and the getters will use the cached value except for their
first invocation.

The NUM_FIELD_SOURCE feature was introduced in 2012 by the very large
commit 7cdd7372 and apparently its costs never were a problem
until the KVP lookup became more costly due to the std::vector
construction and destruction.

libgnucash/engine/qofbook.cpp

@@ -92,6 +92,13 @@ enum
     N_PROPERTIES		/* Just a counter */
 };
 
+static void
+qof_book_option_num_field_source_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"
+
 QOF_GOBJECT_GET_TYPE(QofBook, qof_book, QOF_TYPE_INSTANCE, {});
 QOF_GOBJECT_DISPOSE(qof_book);
 QOF_GOBJECT_FINALIZE(qof_book);
@@ -126,6 +133,15 @@ qof_book_init (QofBook *book)
     book->read_only = FALSE;
     book->session_dirty = FALSE;
     book->version = 0;
+    book->cached_num_field_source_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
+    // a bool member variable instead of a KVP lookup on each getter call.
+    g_signal_connect (G_OBJECT(book),
+                      "notify::" PARAM_NAME_NUM_FIELD_SOURCE,
+                      G_CALLBACK (qof_book_option_num_field_source_changed_cb),
+                      book);
 }
 
 static const std::string str_KVP_OPTION_PATH(KVP_OPTION_PATH);
@@ -301,7 +317,7 @@ qof_book_class_init (QofBookClass *klass)
     g_object_class_install_property
     (gobject_class,
      PROP_OPT_NUM_FIELD_SOURCE,
-     g_param_spec_string("split-action-num-field",
+     g_param_spec_string(PARAM_NAME_NUM_FIELD_SOURCE,
                          "Use Split-Action in the Num Field",
 			 "Scheme true ('t') or NULL. If 't', then the book "
 			 "will put the split action value in the Num field.",
@@ -1002,14 +1018,42 @@ qof_book_use_trading_accounts (const QofBook *book)
 gboolean
 qof_book_use_split_action_for_num_field (const QofBook *book)
 {
-    const char *opt = NULL;
-    qof_instance_get (QOF_INSTANCE (book),
-		      "split-action-num-field", &opt,
-		      NULL);
+    g_assert(book);
+    if (!book->cached_num_field_source_isvalid)
+    {
+        // No cached value? Then do the expensive KVP lookup
+        gboolean result;
+        const char *opt = NULL;
+        qof_instance_get (QOF_INSTANCE (book),
+                          PARAM_NAME_NUM_FIELD_SOURCE, &opt,
+                          NULL);
+
+        if (opt && opt[0] == 't' && opt[1] == 0)
+            result = TRUE;
+        else
+            result = FALSE;
+
+        // We need to const_cast the "book" argument into a non-const pointer,
+        // but as we are dealing only with cache variables, I think this is
+        // understandable enough.
+        const_cast<QofBook*>(book)->cached_num_field_source = result;
+        const_cast<QofBook*>(book)->cached_num_field_source_isvalid = TRUE;
+    }
+    // Value is cached now. Use the cheap variable returning.
+    return book->cached_num_field_source;
+}
 
-    if (opt && opt[0] == 't' && opt[1] == 0)
-        return TRUE;
-    return FALSE;
+// The callback that is called when the KVP option value of
+// "split-action-num-field" changes, so that we mark the cached value as
+// invalid.
+static void
+qof_book_option_num_field_source_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_field_source_isvalid = FALSE;
 }
 
 gboolean qof_book_uses_autoreadonly (const QofBook *book)
@@ -1188,6 +1232,9 @@ qof_book_set_option (QofBook *book, KvpValue *value, GSList *path)
     delete root->set_path(gslist_to_option_path(path), value);
     qof_instance_set_dirty (QOF_INSTANCE (book));
     qof_book_commit_edit (book);
+
+    // Also, mark any cached value as invalid
+    book->cached_num_field_source_isvalid = FALSE;
 }
 
 KvpValue*

libgnucash/engine/qofbook.h

@@ -147,6 +147,13 @@ struct _QofBook
      * except that it provides a nice convenience, avoiding a lookup
      * from the session.  Better solutions welcome ... */
     QofBackend *backend;
+
+    /* A cached value of the OPTION_NAME_NUM_FIELD_SOURCE option value because
+     * it is queried quite a lot, so we want to avoid a KVP lookup on each query
+     */
+    gboolean cached_num_field_source;
+    /* Whether the above cached value is valid. */
+    gboolean cached_num_field_source_isvalid;
 };
 
 struct _QofBookClass