gnucash maint: Register speed-up for large files.
Christian Stimming
cstim at code.gnucash.org
Fri Jun 22 18:00:21 EDT 2018
Updated via https://github.com/Gnucash/gnucash/commit/92ea3ba8 (commit)
from https://github.com/Gnucash/gnucash/commit/f144a8de (commit)
commit 92ea3ba8a60bf4eb19d9b6932fb3ed8b582551a5
Author: Christian Stimming <christian at cstimming.de>
Date: Fri Jun 22 22:17:38 2018 +0200
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.
diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
index 9c3974c..bf54dcc 100644
--- a/libgnucash/engine/qofbook.cpp
+++ b/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*
diff --git a/libgnucash/engine/qofbook.h b/libgnucash/engine/qofbook.h
index 8cc1655..c695de3 100644
--- a/libgnucash/engine/qofbook.h
+++ b/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
Summary of changes:
libgnucash/engine/qofbook.cpp | 63 +++++++++++++++++++++++++++++++++++++------
libgnucash/engine/qofbook.h | 7 +++++
2 files changed, 62 insertions(+), 8 deletions(-)
More information about the gnucash-changes
mailing list