From 00e4790353a6378d09523d7eabb47eb25c4cb23d Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Thu, 7 Jul 2016 16:56:57 -0700 Subject: [PATCH] Have `List` and `Results` fetch their object schema when requested. This avoids the need to eagerly fetch the object schema in order to construct a `List` or `Results`. Instead the work can be deferred until the object schema is requested. Since `List` and `Results` never use the object schema themselves this can avoid unnecessary work in some bindings. --- src/object-store/src/list.cpp | 24 +++++++++++---- src/object-store/src/list.hpp | 6 ++-- src/object-store/src/object_accessor.hpp | 5 ++-- src/object-store/src/results.cpp | 38 +++++++++++++++++------- src/object-store/src/results.hpp | 12 ++++---- src/object-store/tests/list.cpp | 28 ++++++++++------- src/object-store/tests/results.cpp | 22 +++++++------- 7 files changed, 85 insertions(+), 50 deletions(-) diff --git a/src/object-store/src/list.cpp b/src/object-store/src/list.cpp index 8f0c6921..b1dd53e6 100644 --- a/src/object-store/src/list.cpp +++ b/src/object-store/src/list.cpp @@ -20,7 +20,9 @@ #include "impl/list_notifier.hpp" #include "impl/realm_coordinator.hpp" +#include "object_store.hpp" #include "results.hpp" +#include "schema.hpp" #include "shared_realm.hpp" #include "util/format.hpp" @@ -37,13 +39,25 @@ List& List::operator=(const List&) = default; List::List(List&&) = default; List& List::operator=(List&&) = default; -List::List(std::shared_ptr r, const ObjectSchema& s, LinkViewRef l) noexcept +List::List(std::shared_ptr r, LinkViewRef l) noexcept : m_realm(std::move(r)) -, m_object_schema(&s) , m_link_view(std::move(l)) { } +const ObjectSchema& List::get_object_schema() const +{ + verify_attached(); + + if (!m_object_schema) { + auto object_type = ObjectStore::object_type_for_table_name(m_link_view->get_target_table().get_name()); + auto it = m_realm->config().schema->find(object_type); + REALM_ASSERT(it != m_realm->config().schema->end()); + m_object_schema = &*it; + } + return *m_object_schema; +} + Query List::get_query() const { verify_attached(); @@ -172,19 +186,19 @@ void List::delete_all() Results List::sort(SortOrder order) { verify_attached(); - return Results(m_realm, *m_object_schema, m_link_view, util::none, std::move(order)); + return Results(m_realm, m_link_view, util::none, std::move(order)); } Results List::filter(Query q) { verify_attached(); - return Results(m_realm, *m_object_schema, m_link_view, get_query().and_query(std::move(q))); + return Results(m_realm, m_link_view, get_query().and_query(std::move(q))); } Results List::snapshot() const { verify_attached(); - return Results(m_realm, *m_object_schema, m_link_view).snapshot(); + return Results(m_realm, m_link_view).snapshot(); } // These definitions rely on that LinkViews are interned by core diff --git a/src/object-store/src/list.hpp b/src/object-store/src/list.hpp index 47bd0e2c..9ad73ec5 100644 --- a/src/object-store/src/list.hpp +++ b/src/object-store/src/list.hpp @@ -40,7 +40,7 @@ struct SortOrder; class List { public: List() noexcept; - List(std::shared_ptr r, const ObjectSchema& s, LinkViewRef l) noexcept; + List(std::shared_ptr r, LinkViewRef l) noexcept; ~List(); List(const List&); @@ -50,7 +50,7 @@ public: const std::shared_ptr& get_realm() const { return m_realm; } Query get_query() const; - const ObjectSchema& get_object_schema() const { return *m_object_schema; } + const ObjectSchema& get_object_schema() const; size_t get_origin_row_index() const; bool is_valid() const; @@ -108,7 +108,7 @@ public: private: std::shared_ptr m_realm; - const ObjectSchema* m_object_schema; + mutable const ObjectSchema* m_object_schema = nullptr; LinkViewRef m_link_view; _impl::CollectionNotifier::Handle<_impl::CollectionNotifier> m_notifier; diff --git a/src/object-store/src/object_accessor.hpp b/src/object-store/src/object_accessor.hpp index 84d07387..226be5df 100644 --- a/src/object-store/src/object_accessor.hpp +++ b/src/object-store/src/object_accessor.hpp @@ -309,15 +309,14 @@ namespace realm { return Accessor::from_object(ctx, std::move(Object(m_realm, *linkObjectSchema, table->get(m_row.get_link(column))))); } case PropertyType::Array: { - auto arrayObjectSchema = m_realm->config().schema->find(property.object_type); - return Accessor::from_list(ctx, std::move(List(m_realm, *arrayObjectSchema, static_cast(m_row.get_linklist(column))))); + return Accessor::from_list(ctx, std::move(List(m_realm, static_cast(m_row.get_linklist(column))))); } case PropertyType::LinkingObjects: { auto target_object_schema = m_realm->config().schema->find(property.object_type); auto link_property = target_object_schema->property_for_name(property.link_origin_property_name); TableRef table = ObjectStore::table_for_object_type(m_realm->read_group(), target_object_schema->name); auto tv = m_row.get_table()->get_backlink_view(m_row.get_index(), table.get(), link_property->table_column); - Results results(m_realm, *m_object_schema, std::move(tv), {}); + Results results(m_realm, std::move(tv), {}); return Accessor::from_results(ctx, std::move(results)); } } diff --git a/src/object-store/src/results.cpp b/src/object-store/src/results.cpp index 2cdb1bba..19e00ba2 100644 --- a/src/object-store/src/results.cpp +++ b/src/object-store/src/results.cpp @@ -22,6 +22,7 @@ #include "impl/results_notifier.hpp" #include "object_schema.hpp" #include "object_store.hpp" +#include "schema.hpp" #include "util/format.hpp" #include "util/compiler.hpp" @@ -32,9 +33,8 @@ using namespace realm; Results::Results() = default; Results::~Results() = default; -Results::Results(SharedRealm r, const ObjectSchema &o, Query q, SortOrder s) +Results::Results(SharedRealm r, Query q, SortOrder s) : m_realm(std::move(r)) -, m_object_schema(&o) , m_query(std::move(q)) , m_table(m_query.get_table().get()) , m_sort(std::move(s)) @@ -43,17 +43,15 @@ Results::Results(SharedRealm r, const ObjectSchema &o, Query q, SortOrder s) REALM_ASSERT(m_sort.column_indices.size() == m_sort.ascending.size()); } -Results::Results(SharedRealm r, const ObjectSchema &o, Table& table) +Results::Results(SharedRealm r, Table& table) : m_realm(std::move(r)) -, m_object_schema(&o) , m_table(&table) , m_mode(Mode::Table) { } -Results::Results(SharedRealm r, const ObjectSchema& o, LinkViewRef lv, util::Optional q, SortOrder s) +Results::Results(SharedRealm r, LinkViewRef lv, util::Optional q, SortOrder s) : m_realm(std::move(r)) -, m_object_schema(&o) , m_link_view(lv) , m_table(&lv->get_target_table()) , m_sort(std::move(s)) @@ -66,9 +64,8 @@ Results::Results(SharedRealm r, const ObjectSchema& o, LinkViewRef lv, util::Opt } } -Results::Results(SharedRealm r, const ObjectSchema& o, TableView tv, SortOrder s) +Results::Results(SharedRealm r, TableView tv, SortOrder s) : m_realm(std::move(r)) -, m_object_schema(&o) , m_table_view(std::move(tv)) , m_table(&m_table_view.get_parent()) , m_sort(std::move(s)) @@ -158,9 +155,28 @@ size_t Results::size() REALM_UNREACHABLE(); } +const ObjectSchema& Results::get_object_schema() const +{ + validate_read(); + + if (!m_object_schema) { + REALM_ASSERT(m_realm); + auto it = m_realm->config().schema->find(get_object_type()); + REALM_ASSERT(it != m_realm->config().schema->end()); + m_object_schema = &*it; + } + + return *m_object_schema; +} + + StringData Results::get_object_type() const noexcept { - return get_object_schema().name; + if (!m_table) { + return StringData(); + } + + return ObjectStore::object_type_for_table_name(m_table->get_name()); } RowExpr Results::get(size_t row_ndx) @@ -480,12 +496,12 @@ TableView Results::get_tableview() Results Results::sort(realm::SortOrder&& sort) const { REALM_ASSERT(sort.column_indices.size() == sort.ascending.size()); - return Results(m_realm, *m_object_schema, get_query(), std::move(sort)); + return Results(m_realm, get_query(), std::move(sort)); } Results Results::filter(Query&& q) const { - return Results(m_realm, *m_object_schema, get_query().and_query(std::move(q)), m_sort); + return Results(m_realm, get_query().and_query(std::move(q)), m_sort); } Results Results::snapshot() const & diff --git a/src/object-store/src/results.hpp b/src/object-store/src/results.hpp index fe3476b7..9a3d31b6 100644 --- a/src/object-store/src/results.hpp +++ b/src/object-store/src/results.hpp @@ -49,10 +49,10 @@ public: // or a wrapper around a query and a sort order which creates and updates // the tableview as needed Results(); - Results(SharedRealm r, const ObjectSchema& o, Table& table); - Results(SharedRealm r, const ObjectSchema& o, Query q, SortOrder s = {}); - Results(SharedRealm r, const ObjectSchema& o, TableView tv, SortOrder s); - Results(SharedRealm r, const ObjectSchema& o, LinkViewRef lv, util::Optional q = {}, SortOrder s = {}); + Results(SharedRealm r, Table& table); + Results(SharedRealm r, Query q, SortOrder s = {}); + Results(SharedRealm r, TableView tv, SortOrder s); + Results(SharedRealm r, LinkViewRef lv, util::Optional q = {}, SortOrder s = {}); ~Results(); // Results is copyable and moveable @@ -65,7 +65,7 @@ public: SharedRealm get_realm() const { return m_realm; } // Object schema describing the vendored object type - const ObjectSchema &get_object_schema() const { return *m_object_schema; } + const ObjectSchema &get_object_schema() const; // Get a query which will match the same rows as is contained in this Results // Returned query will not be valid if the current mode is Empty @@ -199,7 +199,7 @@ private: }; SharedRealm m_realm; - const ObjectSchema *m_object_schema; + mutable const ObjectSchema *m_object_schema = nullptr; Query m_query; TableView m_table_view; LinkViewRef m_link_view; diff --git a/src/object-store/tests/list.cpp b/src/object-store/tests/list.cpp index fec1f2e0..b98a57f4 100644 --- a/src/object-store/tests/list.cpp +++ b/src/object-store/tests/list.cpp @@ -61,7 +61,7 @@ TEST_CASE("list") { SECTION("add_notification_block()") { CollectionChangeSet change; - List lst(r, *r->config().schema->find("origin"), lv); + List lst(r, lv); auto write = [&](auto&& f) { r->begin_transaction(); @@ -190,7 +190,7 @@ TEST_CASE("list") { auto get_list = [&] { auto r = Realm::get_shared_realm(config); auto lv = r->read_group()->get_table("class_origin")->get_linklist(0, 0); - return List(r, *r->config().schema->find("origin"), lv); + return List(r, lv); }; auto change_list = [&] { r->begin_transaction(); @@ -250,7 +250,7 @@ TEST_CASE("list") { lv2->add(0); r->commit_transaction(); - List lst2(r, *r->config().schema->find("other_origin"), lv2); + List lst2(r, lv2); // Add a callback for list1, advance the version, then add a // callback for list2, so that the notifiers added at each source @@ -299,7 +299,7 @@ TEST_CASE("list") { } SECTION("sorted add_notification_block()") { - List lst(r, *r->config().schema->find("origin"), lv); + List lst(r, lv); Results results = lst.sort({{0}, {false}}); int notification_calls = 0; @@ -355,7 +355,7 @@ TEST_CASE("list") { } SECTION("filtered add_notification_block()") { - List lst(r, *r->config().schema->find("origin"), lv); + List lst(r, lv); Results results = lst.filter(target->where().less(0, 9)); int notification_calls = 0; @@ -420,8 +420,8 @@ TEST_CASE("list") { } SECTION("sort()") { - auto objectschema = &*r->config().schema->find("origin"); - List list(r, *objectschema, lv); + auto objectschema = &*r->config().schema->find("target"); + List list(r, lv); auto results = list.sort({{0}, {false}}); REQUIRE(&results.get_object_schema() == objectschema); @@ -435,8 +435,8 @@ TEST_CASE("list") { } SECTION("filter()") { - auto objectschema = &*r->config().schema->find("origin"); - List list(r, *objectschema, lv); + auto objectschema = &*r->config().schema->find("target"); + List list(r, lv); auto results = list.filter(target->where().greater(0, 5)); REQUIRE(&results.get_object_schema() == objectschema); @@ -449,8 +449,8 @@ TEST_CASE("list") { } SECTION("snapshot()") { - auto objectschema = &*r->config().schema->find("origin"); - List list(r, *objectschema, lv); + auto objectschema = &*r->config().schema->find("target"); + List list(r, lv); auto snapshot = list.snapshot(); REQUIRE(&snapshot.get_object_schema() == objectschema); @@ -478,4 +478,10 @@ TEST_CASE("list") { list.add(0); REQUIRE(snapshot.size() == 10); } + + SECTION("get_object_schema()") { + List list(r, lv); + auto objectschema = &*r->config().schema->find("target"); + REQUIRE(&list.get_object_schema() == objectschema); + } } diff --git a/src/object-store/tests/results.cpp b/src/object-store/tests/results.cpp index 4bc2b15b..18bf1243 100644 --- a/src/object-store/tests/results.cpp +++ b/src/object-store/tests/results.cpp @@ -48,7 +48,7 @@ TEST_CASE("[results] notifications") { table->set_int(0, i, i * 2); r->commit_transaction(); - Results results(r, *config.schema->find("object"), table->where().greater(0, 0).less(0, 10)); + Results results(r, table->where().greater(0, 0).less(0, 10)); SECTION("unsorted notifications") { int notification_calls = 0; @@ -433,7 +433,7 @@ TEST_CASE("[results] async error handling") { auto r = Realm::get_shared_realm(config); auto coordinator = _impl::RealmCoordinator::get_existing_coordinator(config.path); - Results results(r, *config.schema->find("object"), *r->read_group()->get_table("class_object")); + Results results(r, *r->read_group()->get_table("class_object")); class OpenFileLimiter { public: @@ -547,7 +547,7 @@ TEST_CASE("[results] notifications after move") { auto r = Realm::get_shared_realm(config); auto table = r->read_group()->get_table("class_object"); - auto results = std::make_unique(r, *config.schema->find("object"), *table); + auto results = std::make_unique(r, *table); int notification_calls = 0; auto token = results->add_notification_callback([&](CollectionChangeSet, std::exception_ptr err) { @@ -596,7 +596,7 @@ TEST_CASE("[results] error messages") { auto r = Realm::get_shared_realm(config); auto table = r->read_group()->get_table("class_object"); - Results results(r, *config.schema->find("object"), *table); + Results results(r, *table); r->begin_transaction(); table->add_empty_row(); @@ -642,7 +642,7 @@ TEST_CASE("results: snapshots") { SECTION("snapshot of Results based on Table") { auto table = r->read_group()->get_table("class_object"); - Results results(r, *config.schema->find("object"), *table); + Results results(r, *table); { // A newly-added row should not appear in the snapshot. @@ -688,7 +688,7 @@ TEST_CASE("results: snapshots") { }); LinkViewRef lv = object->get_linklist(1, 0); - Results results(r, *config.schema->find("linked to object"), lv); + Results results(r, lv); { // A newly-added row should not appear in the snapshot. @@ -734,7 +734,7 @@ TEST_CASE("results: snapshots") { SECTION("snapshot of Results based on Query") { auto table = r->read_group()->get_table("class_object"); Query q = table->column(0) > 0; - Results results(r, *config.schema->find("object"), std::move(q)); + Results results(r, std::move(q)); { // A newly-added row should not appear in the snapshot. @@ -780,7 +780,7 @@ TEST_CASE("results: snapshots") { SECTION("snapshot of Results based on TableView from query") { auto table = r->read_group()->get_table("class_object"); Query q = table->column(0) > 0; - Results results(r, *config.schema->find("object"), q.find_all(), {}); + Results results(r, q.find_all(), {}); { // A newly-added row should not appear in the snapshot. @@ -832,7 +832,7 @@ TEST_CASE("results: snapshots") { }); TableView backlinks = linked_to->get_backlink_view(0, object.get(), 1); - Results results(r, *config.schema->find("object"), std::move(backlinks), {}); + Results results(r, std::move(backlinks), {}); auto lv = object->get_linklist(1, object->add_empty_row()); @@ -882,7 +882,7 @@ TEST_CASE("results: snapshots") { SECTION("snapshot of Results with notification callback registered") { auto table = r->read_group()->get_table("class_object"); Query q = table->column(0) > 0; - Results results(r, *config.schema->find("object"), q.find_all(), {}); + Results results(r, q.find_all(), {}); auto token = results.add_notification_callback([&](CollectionChangeSet, std::exception_ptr err) { REQUIRE_FALSE(err); @@ -909,7 +909,7 @@ TEST_CASE("results: snapshots") { SECTION("adding notification callback to snapshot throws") { auto table = r->read_group()->get_table("class_object"); Query q = table->column(0) > 0; - Results results(r, *config.schema->find("object"), q.find_all(), {}); + Results results(r, q.find_all(), {}); auto snapshot = results.snapshot(); CHECK_THROWS(snapshot.add_notification_callback([](CollectionChangeSet, std::exception_ptr) {})); }