Skip to content

Commit

Permalink
Make Obj trivial and add a separate ObjCollectionParent type
Browse files Browse the repository at this point in the history 
Giving Obj a virtual base class turns out to have a meaningful performance
impact.
  • Loading branch information
@tgoyne
tgoyne committed Mar 7, 2024
1 parent c911c9f commit eabaac3
Show file tree
Hide file tree
Showing 19 changed files with 384 additions and 293 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Fixed an issue when removing items from a LnkLst that could result in invalidated links becoming visable which could cause crashes or exceptions when accessing those list items later on. This affects sync Realms where another client had previously removed a link in a linklist that has over 1000 links in it, and then further local removals from the same list caused the list to have fewer than 1000 items. ([#7414](https://github.com/realm/realm-core/pull/7414), since v10.0.0)
* Query lists vs lists if the property to check is a path with wildcards would not give correct result. This has for a long time also been a problem for queries with linklist properties in the path ([#7393](https://github.com/realm/realm-core/issues/7393), since v14.0.0)
* Fix a data race in change notification delivery when running at debug log level ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0).
* Fix a 10-15% performance regression when reading data from the Realm resulting from Obj being made a non-trivial type ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0).

### Breaking changes
* The fix of querying involving multiple lists may cause tests that depended on the broken beharior to fail.
Expand Down
8 changes: 4 additions & 4 deletions src/realm/collection.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
using Interface::get_target_table;

protected:
Obj m_obj_mem;
ObjCollectionParent m_obj_mem;
std::shared_ptr<CollectionParent> m_col_parent;
CollectionParent::Index m_index;
ColKey m_col_key;
Expand Down Expand Up @@ -724,19 +724,19 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
void set_backlink(ColKey col_key, ObjLink new_link) const
{
check_parent();
m_parent->set_backlink(col_key, new_link);
m_parent->get_object().set_backlink(col_key, new_link);
}
// Used when replacing a link, return true if CascadeState contains objects to remove
bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const
{
check_parent();
return m_parent->replace_backlink(col_key, old_link, new_link, state);
return m_parent->get_object().replace_backlink(col_key, old_link, new_link, state);
}
// Used when removing a backlink, return true if CascadeState contains objects to remove
bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const
{
check_parent();
return m_parent->remove_backlink(col_key, old_link, state);
return m_parent->get_object().remove_backlink(col_key, old_link, state);
}

/// Reset the accessor's tracking of the content version. Derived classes
Expand Down
239 changes: 58 additions & 181 deletions src/realm/collection_parent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,212 +79,81 @@ void CollectionParent::check_level() const
throw LogicError(ErrorCodes::LimitExceeded, "Max nesting level reached");
}
}
void CollectionParent::set_backlink(ColKey col_key, ObjLink new_link) const
{
if (new_link && new_link.get_obj_key()) {
auto t = get_table();
auto target_table = t->get_parent_group()->get_table(new_link.get_table_key());
ColKey backlink_col_key;
auto type = col_key.get_type();
if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) {
// This may modify the target table
backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key());
// it is possible that this was a link to the same table and that adding a backlink column has
// caused the need to update this object as well.
update_if_needed();
}
else {
backlink_col_key = t->get_opposite_column(col_key);
}
auto obj_key = new_link.get_obj_key();
auto target_obj = obj_key.is_unresolved() ? target_table->try_get_tombstone(obj_key)
: target_table->try_get_object(obj_key);
if (!target_obj) {
throw InvalidArgument(ErrorCodes::KeyNotFound, "Target object not found");
}
target_obj.add_backlink(backlink_col_key, get_object().get_key());
}
}

bool CollectionParent::replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const
template <typename Base, template <typename> typename Collection, typename LinkCol>
std::unique_ptr<Base> create_collection(ColKey col_key, size_t level)
{
bool recurse = remove_backlink(col_key, old_link, state);
set_backlink(col_key, new_link);

return recurse;
}

bool CollectionParent::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const
{
if (old_link && old_link.get_obj_key()) {
auto t = get_table();
REALM_ASSERT(t->valid_column(col_key));
ObjKey old_key = old_link.get_obj_key();
auto target_obj = t->get_parent_group()->get_object(old_link);
TableRef target_table = target_obj.get_table();
ColKey backlink_col_key;
auto type = col_key.get_type();
if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) {
backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key());
}
else {
backlink_col_key = t->get_opposite_column(col_key);
}

bool strong_links = target_table->is_embedded();
bool is_unres = old_key.is_unresolved();

bool last_removed = target_obj.remove_one_backlink(backlink_col_key, get_object().get_key()); // Throws
if (is_unres) {
if (last_removed) {
// Check is there are more backlinks
if (!target_obj.has_backlinks(false)) {
// Tombstones can be erased right away - there is no cascading effect
target_table->m_tombstones->erase(old_key, state);
}
}
}
else {
return state.enqueue_for_cascade(target_obj, strong_links, last_removed);
}
}

return false;
}

LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key) const
{
auto table = get_table();
auto attr = table->get_column_attr(col_key);
REALM_ASSERT(attr.test(col_attr_List) || attr.test(col_attr_Nullable));
bool nullable = attr.test(col_attr_Nullable);

switch (table->get_column_type(col_key)) {
case type_Int: {
bool nullable = col_key.get_attrs().test(col_attr_Nullable);
switch (col_key.get_type()) {
case col_type_Int:
if (nullable)
return std::make_unique<Lst<util::Optional<Int>>>(col_key);
else
return std::make_unique<Lst<Int>>(col_key);
}
case type_Bool: {
return std::make_unique<Collection<util::Optional<Int>>>(col_key);
return std::make_unique<Collection<Int>>(col_key);
case col_type_Bool:
if (nullable)
return std::make_unique<Lst<util::Optional<Bool>>>(col_key);
else
return std::make_unique<Lst<Bool>>(col_key);
}
case type_Float: {
return std::make_unique<Collection<util::Optional<Bool>>>(col_key);
return std::make_unique<Collection<Bool>>(col_key);
case col_type_Float:
if (nullable)
return std::make_unique<Lst<util::Optional<Float>>>(col_key);
else
return std::make_unique<Lst<Float>>(col_key);
}
case type_Double: {
return std::make_unique<Collection<util::Optional<Float>>>(col_key);
return std::make_unique<Collection<Float>>(col_key);
case col_type_Double:
if (nullable)
return std::make_unique<Lst<util::Optional<Double>>>(col_key);
else
return std::make_unique<Lst<Double>>(col_key);
}
case type_String: {
return std::make_unique<Lst<String>>(col_key);
}
case type_Binary: {
return std::make_unique<Lst<Binary>>(col_key);
}
case type_Timestamp: {
return std::make_unique<Lst<Timestamp>>(col_key);
}
case type_Decimal: {
return std::make_unique<Lst<Decimal128>>(col_key);
}
case type_ObjectId: {
return std::make_unique<Collection<util::Optional<Double>>>(col_key);
return std::make_unique<Collection<Double>>(col_key);
case col_type_String:
return std::make_unique<Collection<String>>(col_key);
case col_type_Binary:
return std::make_unique<Collection<Binary>>(col_key);
case col_type_Timestamp:
return std::make_unique<Collection<Timestamp>>(col_key);
case col_type_Decimal:
return std::make_unique<Collection<Decimal128>>(col_key);
case col_type_ObjectId:
if (nullable)
return std::make_unique<Lst<util::Optional<ObjectId>>>(col_key);
else
return std::make_unique<Lst<ObjectId>>(col_key);
}
case type_UUID: {
return std::make_unique<Collection<util::Optional<ObjectId>>>(col_key);
return std::make_unique<Collection<ObjectId>>(col_key);
case col_type_UUID:
if (nullable)
return std::make_unique<Lst<util::Optional<UUID>>>(col_key);
else
return std::make_unique<Lst<UUID>>(col_key);
}
case type_TypedLink: {
return std::make_unique<Lst<ObjLink>>(col_key);
}
case type_Mixed: {
return std::make_unique<Lst<Mixed>>(col_key, get_level() + 1);
}
case type_Link:
return std::make_unique<LnkLst>(col_key);
return std::make_unique<Collection<util::Optional<UUID>>>(col_key);
return std::make_unique<Collection<UUID>>(col_key);
case col_type_TypedLink:
return std::make_unique<Collection<ObjLink>>(col_key);
case col_type_Mixed:
return std::make_unique<Collection<Mixed>>(col_key, level + 1);
case col_type_Link:
return std::make_unique<LinkCol>(col_key);
default:
REALM_TERMINATE("Unsupported column type.");
}
REALM_TERMINATE("Unsupported column type");
}

SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key) const
LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key, size_t level)
{
auto table = get_table();
auto attr = table->get_column_attr(col_key);
REALM_ASSERT(attr.test(col_attr_Set));
bool nullable = attr.test(col_attr_Nullable);
REALM_ASSERT(col_key.get_attrs().test(col_attr_List) || col_key.get_type() == col_type_Mixed);
return create_collection<LstBase, Lst, LnkLst>(col_key, level);
}

switch (table->get_column_type(col_key)) {
case type_Int:
if (nullable)
return std::make_unique<Set<util::Optional<Int>>>(col_key);
return std::make_unique<Set<Int>>(col_key);
case type_Bool:
if (nullable)
return std::make_unique<Set<util::Optional<Bool>>>(col_key);
return std::make_unique<Set<Bool>>(col_key);
case type_Float:
if (nullable)
return std::make_unique<Set<util::Optional<Float>>>(col_key);
return std::make_unique<Set<Float>>(col_key);
case type_Double:
if (nullable)
return std::make_unique<Set<util::Optional<Double>>>(col_key);
return std::make_unique<Set<Double>>(col_key);
case type_String:
return std::make_unique<Set<String>>(col_key);
case type_Binary:
return std::make_unique<Set<Binary>>(col_key);
case type_Timestamp:
return std::make_unique<Set<Timestamp>>(col_key);
case type_Decimal:
return std::make_unique<Set<Decimal128>>(col_key);
case type_ObjectId:
if (nullable)
return std::make_unique<Set<util::Optional<ObjectId>>>(col_key);
return std::make_unique<Set<ObjectId>>(col_key);
case type_UUID:
if (nullable)
return std::make_unique<Set<util::Optional<UUID>>>(col_key);
return std::make_unique<Set<UUID>>(col_key);
case type_TypedLink:
return std::make_unique<Set<ObjLink>>(col_key);
case type_Mixed:
return std::make_unique<Set<Mixed>>(col_key);
case type_Link:
return std::make_unique<LnkSet>(col_key);
}
REALM_TERMINATE("Unsupported column type.");
SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key, size_t level)
{
REALM_ASSERT(col_key.get_attrs().test(col_attr_Set));
return create_collection<SetBase, Set, LnkSet>(col_key, level);
}

CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key) const
CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key, size_t level)
{
if (col_key.is_list()) {
return get_listbase_ptr(col_key);
return get_listbase_ptr(col_key, level);
}
else if (col_key.is_set()) {
return get_setbase_ptr(col_key);
return get_setbase_ptr(col_key, level);
}
else if (col_key.is_dictionary()) {
return std::make_unique<Dictionary>(col_key, get_level() + 1);
return std::make_unique<Dictionary>(col_key, level + 1);
}
return {};
}


int64_t CollectionParent::generate_key(size_t sz)
{
static std::mt19937 gen32;
Expand All @@ -307,5 +176,13 @@ int64_t CollectionParent::generate_key(size_t sz)
return key;
}

void CollectionParent::set_key(BPlusTreeMixed& tree, size_t index)
{
int64_t key = generate_key(tree.size());
while (tree.find_key(key) != realm::not_found) {
key++;
}
tree.set_key(index, key);
}

} // namespace realm
26 changes: 11 additions & 15 deletions src/realm/collection_parent.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@
#define REALM_COLLECTION_PARENT_HPP

#include <realm/alloc.hpp>
#include <realm/table_ref.hpp>
#include <realm/path.hpp>
#include <realm/mixed.hpp>
#include <realm/path.hpp>
#include <realm/table_ref.hpp>

namespace realm {

class Obj;
class Replication;
class CascadeState;
class BPlusTreeMixed;

class Collection;
class CollectionBase;
Expand Down Expand Up @@ -95,6 +96,13 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
/// Get table of owning object
virtual TableRef get_table() const noexcept = 0;

static LstBasePtr get_listbase_ptr(ColKey col_key, size_t level);
static SetBasePtr get_setbase_ptr(ColKey col_key, size_t level);
static CollectionBasePtr get_collection_ptr(ColKey col_key, size_t level);

static int64_t generate_key(size_t sz);
static void set_key(BPlusTreeMixed& tree, size_t index);

protected:
friend class Collection;
template <class>
Expand Down Expand Up @@ -129,20 +137,8 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
/// Set the top ref in parent
virtual void set_collection_ref(Index, ref_type ref, CollectionType) = 0;

/// Get the counter which is incremented whenever the root Obj is updated.
virtual uint32_t parent_version() const noexcept = 0;

// Used when inserting a new link. You will not remove existing links in this process
void set_backlink(ColKey col_key, ObjLink new_link) const;
// Used when replacing a link, return true if CascadeState contains objects to remove
bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const;
// Used when removing a backlink, return true if CascadeState contains objects to remove
bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const;

LstBasePtr get_listbase_ptr(ColKey col_key) const;
SetBasePtr get_setbase_ptr(ColKey col_key) const;
CollectionBasePtr get_collection_ptr(ColKey col_key) const;

static int64_t generate_key(size_t sz);
};

} // namespace realm
Expand Down
7 changes: 1 addition & 6 deletions src/realm/dictionary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,7 @@ void Dictionary::insert_collection(const PathElement& path_elem, CollectionType
if (!old_val || *old_val != new_val) {
m_values->ensure_keys();
auto [it, inserted] = insert(path_elem.get_key(), new_val);
int64_t key = generate_key(size());
while (m_values->find_key(key) != realm::not_found) {
key++;
}
m_values->set_key(it.index(), key);
set_key(*m_values, it.index());
}
}

Expand Down Expand Up @@ -682,7 +678,6 @@ void Dictionary::ensure_created()
{
constexpr bool allow_create = true;
if (do_update_if_needed(allow_create) == UpdateStatus::Detached) {
// FIXME: untested
throw StaleAccessor("Dictionary no longer exists");
}
}
Expand Down

0 comments on commit eabaac3

Please sign in to comment.