From d191bb40fe8fc3fd27cdd54c8117ffb87f1e21dd Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 15 Jun 2026 09:14:17 +0800 Subject: [PATCH] [qofbook.cpp] hash_of_collections is a flat_map note the key is a std::string instead of the interned string_cache const char*. Each flat_map cell will be pair. --- .../test/gtest-import-backend.cpp | 4 ++ libgnucash/engine/mocks/gmock-qofbook.h | 1 - libgnucash/engine/qofbook-p.hpp | 15 ++++- libgnucash/engine/qofbook.cpp | 62 +++---------------- libgnucash/engine/test/test-qofbook.cpp | 12 ++-- 5 files changed, 32 insertions(+), 62 deletions(-) diff --git a/gnucash/import-export/test/gtest-import-backend.cpp b/gnucash/import-export/test/gtest-import-backend.cpp index cca303c106..03309c6876 100644 --- a/gnucash/import-export/test/gtest-import-backend.cpp +++ b/gnucash/import-export/test/gtest-import-backend.cpp @@ -124,6 +124,10 @@ gnc_commodity_equiv(const gnc_commodity * a, const gnc_commodity * b) return TRUE; } +// fake function from qofid.cpp +void qof_collection_destroy (QofCollection *col) +{ +} /* required fake functions from app-utils sources, which should not be linked to the test application */ diff --git a/libgnucash/engine/mocks/gmock-qofbook.h b/libgnucash/engine/mocks/gmock-qofbook.h index 04619ad6c6..690dbad5f8 100644 --- a/libgnucash/engine/mocks/gmock-qofbook.h +++ b/libgnucash/engine/mocks/gmock-qofbook.h @@ -26,7 +26,6 @@ class QofMockBook : public QofBook public: QofMockBook() { - hash_of_collections = nullptr; data_tables = nullptr; data_table_finalizers = nullptr; diff --git a/libgnucash/engine/qofbook-p.hpp b/libgnucash/engine/qofbook-p.hpp index e63b357dfa..ab668557b1 100644 --- a/libgnucash/engine/qofbook-p.hpp +++ b/libgnucash/engine/qofbook-p.hpp @@ -42,6 +42,17 @@ #include "qofid-p.h" #include "qofinstance-p.h" +#include +#include +#include + +struct QofCollectionDeleter +{ + void operator()(QofCollection* col) const noexcept { qof_collection_destroy (col); } +}; + +using QofCollectionPtr = std::unique_ptr; +using CollectionMap = boost::container::flat_map; struct QofBook { @@ -74,7 +85,7 @@ struct QofBook * belonging to this book, with their pointers to the respective * objects. This allows a lookup of objects based on their guid. */ - GHashTable * hash_of_collections; + CollectionMap hash_of_collections; /* In order to store arbitrary data, for extensibility, add a table * that will be used to hold arbitrary pointers. @@ -136,7 +147,7 @@ typedef struct QofBookDirtyCB (*get_dirty_cb)(const QofBook*); void (*set_shutting_down)(QofBook*, gboolean); gpointer (*get_dirty_data)(const QofBook*); - GHashTable* (*get_collections)(const QofBook*); + const CollectionMap& (*get_collections)(const QofBook*); GHashTable* (*get_data_tables)(const QofBook*); GHashTable* (*get_data_table_finalizers)(const QofBook*); char (*get_book_open)(const QofBook*); diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp index 27a262c9e4..e5e86d9eae 100644 --- a/libgnucash/engine/qofbook.cpp +++ b/libgnucash/engine/qofbook.cpp @@ -96,20 +96,12 @@ QOF_GOBJECT_FINALIZE(qof_book); /* ====================================================================== */ /* constructor / destructor */ -static void coll_destroy(gpointer col) -{ - qof_collection_destroy((QofCollection *) col); -} - static void qof_book_init (QofBook *book) { if (!book) return; - book->hash_of_collections = g_hash_table_new_full( - g_str_hash, g_str_equal, - (GDestroyNotify)qof_string_cache_remove, /* key_destroy_func */ - coll_destroy); /* value_destroy_func */ + new (&book->hash_of_collections) CollectionMap(); qof_instance_init_data (&book->inst, QOF_ID_BOOK, book); @@ -330,9 +322,7 @@ destroy_lot(QofInstance *inst, [[maybe_unused]]void* data) void qof_book_destroy (QofBook *book) { - GHashTable* cols; - - if (!book || !book->hash_of_collections) return; + if (!book) return; ENTER ("book=%p", book); book->shutting_down = TRUE; @@ -356,15 +346,7 @@ qof_book_destroy (QofBook *book) book->data_tables = nullptr; /* qof_instance_release (&book->inst); */ - - /* Note: we need to save this hashtable until after we remove ourself - * from it, otherwise we'll crash in our dispose() function when we - * DO remove ourself from the collection but the collection had already - * been destroyed. - */ - cols = book->hash_of_collections; g_object_unref (book); - g_hash_table_destroy (cols); LEAVE ("book=%p", book); } @@ -520,49 +502,23 @@ qof_book_empty(const QofBook *book) QofCollection * qof_book_get_collection (const QofBook *book, QofIdType entity_type) { - QofCollection *col; - if (!book || !entity_type) return nullptr; - col = static_cast(g_hash_table_lookup (book->hash_of_collections, entity_type)); - if (!col) - { - col = qof_collection_new (entity_type); - g_hash_table_insert( - book->hash_of_collections, - (gpointer)qof_string_cache_insert(entity_type), col); - } - return col; -} - -struct _iterate -{ - QofCollectionForeachCB fn; - gpointer data; -}; - -static void -foreach_cb (G_GNUC_UNUSED gpointer key, gpointer item, gpointer arg) -{ - struct _iterate *iter = static_cast<_iterate*>(arg); - QofCollection *col = static_cast(item); - - iter->fn (col, iter->data); + auto& ptr = const_cast(book)->hash_of_collections[entity_type]; + if (!ptr) + ptr.reset (qof_collection_new (entity_type)); + return ptr.get (); } void qof_book_foreach_collection (const QofBook *book, QofCollectionForeachCB cb, gpointer user_data) { - struct _iterate iter; - g_return_if_fail (book); g_return_if_fail (cb); - iter.fn = cb; - iter.data = user_data; - - g_hash_table_foreach (book->hash_of_collections, foreach_cb, &iter); + for (auto& [name, col] : book->hash_of_collections) + cb (col.get(), user_data); } /* ====================================================================== */ @@ -1415,7 +1371,7 @@ static gboolean get_read_only(const QofBook *book){ return book->read_only; } static QofBookDirtyCB get_dirty_cb(const QofBook *book){ return book->dirty_cb; } static void set_shutting_down(QofBook *book, gboolean state){ book->shutting_down = state; } static gpointer get_dirty_data(const QofBook *book){ return book->dirty_data; } -static GHashTable* get_collections(const QofBook *book){ return book->hash_of_collections; } +static const CollectionMap& get_collections(const QofBook *book){ return book->hash_of_collections; } static GHashTable* get_data_tables(const QofBook *book){ return book->data_tables; } static GHashTable* get_data_table_finalizers(const QofBook *book){ return book->data_table_finalizers; } static char get_book_open(const QofBook *book){ return book->book_open; } diff --git a/libgnucash/engine/test/test-qofbook.cpp b/libgnucash/engine/test/test-qofbook.cpp index eb90406251..365d1e31a2 100644 --- a/libgnucash/engine/test/test-qofbook.cpp +++ b/libgnucash/engine/test/test-qofbook.cpp @@ -780,13 +780,13 @@ test_book_get_collection( Fixture *fixture, gconstpointer pData ) g_assert_true( qof_book_get_collection( fixture->book, NULL ) == NULL ); g_test_message( "Testing when collection does not exist" ); - g_assert_true (test_funcs->get_collections (fixture->book) != NULL ); - g_assert_true (g_hash_table_lookup (test_funcs->get_collections (fixture->book), my_type ) == NULL); + const auto& coll{test_funcs->get_collections (fixture->book)}; + g_assert_true (coll.find (my_type) == coll.end()); m_col = qof_book_get_collection( fixture->book, my_type ); g_assert_true( m_col != NULL ); g_test_message( "Testing with existing collection" ); - g_assert_true (g_hash_table_lookup (test_funcs->get_collections (fixture->book), my_type ) != NULL); + g_assert_true (coll.find (my_type) != coll.end()); m_col2 = qof_book_get_collection( fixture->book, my_type ); g_assert_true( m_col2 != NULL ); g_assert_true( m_col == m_col2 ); @@ -965,11 +965,11 @@ test_book_new_destroy( void ) g_assert_true( QOF_IS_BOOK( book ) ); g_test_message( "Testing book initial setup" ); - g_assert_true (test_funcs->get_collections (book) != NULL); + const auto& coll{test_funcs->get_collections (book)}; g_assert_true( test_funcs->get_data_tables (book) ); g_assert_true( test_funcs->get_data_table_finalizers (book) ); - g_assert_cmpint( g_hash_table_size( test_funcs->get_collections (book) ), == , 1 ); - g_assert_true( g_hash_table_lookup (test_funcs->get_collections (book), QOF_ID_BOOK ) != NULL ); + g_assert_cmpint (coll.size(), == , 1 ); + g_assert_true (coll.find (QOF_ID_BOOK) != coll.end()); g_assert_cmpint( g_hash_table_size( test_funcs->get_data_tables (book) ), == , 0 ); g_assert_cmpint( g_hash_table_size( test_funcs->get_data_table_finalizers (book) ), == , 0 ); g_assert_true (qof_book_is_open(book));