Skip to content

Commit eabaac3

Browse files
tgoynetgoyne
committed
Make Obj trivial and add a separate ObjCollectionParent type
Giving Obj a virtual base class turns out to have a meaningful performance impact.
1 parent c911c9f commit eabaac3

19 files changed

+384
-293
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* 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)
1313
* 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)
1414
* 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).
15+
* 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).
1516

1617
### Breaking changes
1718
* The fix of querying involving multiple lists may cause tests that depended on the broken beharior to fail.

‎src/realm/collection.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
571571
using Interface::get_target_table;
572572

573573
protected:
574-
Obj m_obj_mem;
574+
ObjCollectionParent m_obj_mem;
575575
std::shared_ptr<CollectionParent> m_col_parent;
576576
CollectionParent::Index m_index;
577577
ColKey m_col_key;
@@ -724,19 +724,19 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
724724
void set_backlink(ColKey col_key, ObjLink new_link) const
725725
{
726726
check_parent();
727-
m_parent->set_backlink(col_key, new_link);
727+
m_parent->get_object().set_backlink(col_key, new_link);
728728
}
729729
// Used when replacing a link, return true if CascadeState contains objects to remove
730730
bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const
731731
{
732732
check_parent();
733-
return m_parent->replace_backlink(col_key, old_link, new_link, state);
733+
return m_parent->get_object().replace_backlink(col_key, old_link, new_link, state);
734734
}
735735
// Used when removing a backlink, return true if CascadeState contains objects to remove
736736
bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const
737737
{
738738
check_parent();
739-
return m_parent->remove_backlink(col_key, old_link, state);
739+
return m_parent->get_object().remove_backlink(col_key, old_link, state);
740740
}
741741

742742
/// Reset the accessor's tracking of the content version. Derived classes

‎src/realm/collection_parent.cpp‎

Lines changed: 58 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -79,212 +79,81 @@ void CollectionParent::check_level() const
7979
throw LogicError(ErrorCodes::LimitExceeded, "Max nesting level reached");
8080
}
8181
}
82-
void CollectionParent::set_backlink(ColKey col_key, ObjLink new_link) const
83-
{
84-
if (new_link && new_link.get_obj_key()) {
85-
auto t = get_table();
86-
auto target_table = t->get_parent_group()->get_table(new_link.get_table_key());
87-
ColKey backlink_col_key;
88-
auto type = col_key.get_type();
89-
if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) {
90-
// This may modify the target table
91-
backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key());
92-
// it is possible that this was a link to the same table and that adding a backlink column has
93-
// caused the need to update this object as well.
94-
update_if_needed();
95-
}
96-
else {
97-
backlink_col_key = t->get_opposite_column(col_key);
98-
}
99-
auto obj_key = new_link.get_obj_key();
100-
auto target_obj = obj_key.is_unresolved() ? target_table->try_get_tombstone(obj_key)
101-
: target_table->try_get_object(obj_key);
102-
if (!target_obj) {
103-
throw InvalidArgument(ErrorCodes::KeyNotFound, "Target object not found");
104-
}
105-
target_obj.add_backlink(backlink_col_key, get_object().get_key());
106-
}
107-
}
10882

109-
bool CollectionParent::replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const
83+
template <typename Base, template <typename> typename Collection, typename LinkCol>
84+
std::unique_ptr<Base> create_collection(ColKey col_key, size_t level)
11085
{
111-
bool recurse = remove_backlink(col_key, old_link, state);
112-
set_backlink(col_key, new_link);
113-
114-
return recurse;
115-
}
116-
117-
bool CollectionParent::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const
118-
{
119-
if (old_link && old_link.get_obj_key()) {
120-
auto t = get_table();
121-
REALM_ASSERT(t->valid_column(col_key));
122-
ObjKey old_key = old_link.get_obj_key();
123-
auto target_obj = t->get_parent_group()->get_object(old_link);
124-
TableRef target_table = target_obj.get_table();
125-
ColKey backlink_col_key;
126-
auto type = col_key.get_type();
127-
if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) {
128-
backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key());
129-
}
130-
else {
131-
backlink_col_key = t->get_opposite_column(col_key);
132-
}
133-
134-
bool strong_links = target_table->is_embedded();
135-
bool is_unres = old_key.is_unresolved();
136-
137-
bool last_removed = target_obj.remove_one_backlink(backlink_col_key, get_object().get_key()); // Throws
138-
if (is_unres) {
139-
if (last_removed) {
140-
// Check is there are more backlinks
141-
if (!target_obj.has_backlinks(false)) {
142-
// Tombstones can be erased right away - there is no cascading effect
143-
target_table->m_tombstones->erase(old_key, state);
144-
}
145-
}
146-
}
147-
else {
148-
return state.enqueue_for_cascade(target_obj, strong_links, last_removed);
149-
}
150-
}
151-
152-
return false;
153-
}
154-
155-
LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key) const
156-
{
157-
auto table = get_table();
158-
auto attr = table->get_column_attr(col_key);
159-
REALM_ASSERT(attr.test(col_attr_List) || attr.test(col_attr_Nullable));
160-
bool nullable = attr.test(col_attr_Nullable);
161-
162-
switch (table->get_column_type(col_key)) {
163-
case type_Int: {
86+
bool nullable = col_key.get_attrs().test(col_attr_Nullable);
87+
switch (col_key.get_type()) {
88+
case col_type_Int:
16489
if (nullable)
165-
return std::make_unique<Lst<util::Optional<Int>>>(col_key);
166-
else
167-
return std::make_unique<Lst<Int>>(col_key);
168-
}
169-
case type_Bool: {
90+
return std::make_unique<Collection<util::Optional<Int>>>(col_key);
91+
return std::make_unique<Collection<Int>>(col_key);
92+
case col_type_Bool:
17093
if (nullable)
171-
return std::make_unique<Lst<util::Optional<Bool>>>(col_key);
172-
else
173-
return std::make_unique<Lst<Bool>>(col_key);
174-
}
175-
case type_Float: {
94+
return std::make_unique<Collection<util::Optional<Bool>>>(col_key);
95+
return std::make_unique<Collection<Bool>>(col_key);
96+
case col_type_Float:
17697
if (nullable)
177-
return std::make_unique<Lst<util::Optional<Float>>>(col_key);
178-
else
179-
return std::make_unique<Lst<Float>>(col_key);
180-
}
181-
case type_Double: {
98+
return std::make_unique<Collection<util::Optional<Float>>>(col_key);
99+
return std::make_unique<Collection<Float>>(col_key);
100+
case col_type_Double:
182101
if (nullable)
183-
return std::make_unique<Lst<util::Optional<Double>>>(col_key);
184-
else
185-
return std::make_unique<Lst<Double>>(col_key);
186-
}
187-
case type_String: {
188-
return std::make_unique<Lst<String>>(col_key);
189-
}
190-
case type_Binary: {
191-
return std::make_unique<Lst<Binary>>(col_key);
192-
}
193-
case type_Timestamp: {
194-
return std::make_unique<Lst<Timestamp>>(col_key);
195-
}
196-
case type_Decimal: {
197-
return std::make_unique<Lst<Decimal128>>(col_key);
198-
}
199-
case type_ObjectId: {
102+
return std::make_unique<Collection<util::Optional<Double>>>(col_key);
103+
return std::make_unique<Collection<Double>>(col_key);
104+
case col_type_String:
105+
return std::make_unique<Collection<String>>(col_key);
106+
case col_type_Binary:
107+
return std::make_unique<Collection<Binary>>(col_key);
108+
case col_type_Timestamp:
109+
return std::make_unique<Collection<Timestamp>>(col_key);
110+
case col_type_Decimal:
111+
return std::make_unique<Collection<Decimal128>>(col_key);
112+
case col_type_ObjectId:
200113
if (nullable)
201-
return std::make_unique<Lst<util::Optional<ObjectId>>>(col_key);
202-
else
203-
return std::make_unique<Lst<ObjectId>>(col_key);
204-
}
205-
case type_UUID: {
114+
return std::make_unique<Collection<util::Optional<ObjectId>>>(col_key);
115+
return std::make_unique<Collection<ObjectId>>(col_key);
116+
case col_type_UUID:
206117
if (nullable)
207-
return std::make_unique<Lst<util::Optional<UUID>>>(col_key);
208-
else
209-
return std::make_unique<Lst<UUID>>(col_key);
210-
}
211-
case type_TypedLink: {
212-
return std::make_unique<Lst<ObjLink>>(col_key);
213-
}
214-
case type_Mixed: {
215-
return std::make_unique<Lst<Mixed>>(col_key, get_level() + 1);
216-
}
217-
case type_Link:
218-
return std::make_unique<LnkLst>(col_key);
118+
return std::make_unique<Collection<util::Optional<UUID>>>(col_key);
119+
return std::make_unique<Collection<UUID>>(col_key);
120+
case col_type_TypedLink:
121+
return std::make_unique<Collection<ObjLink>>(col_key);
122+
case col_type_Mixed:
123+
return std::make_unique<Collection<Mixed>>(col_key, level + 1);
124+
case col_type_Link:
125+
return std::make_unique<LinkCol>(col_key);
126+
default:
127+
REALM_TERMINATE("Unsupported column type.");
219128
}
220-
REALM_TERMINATE("Unsupported column type");
221129
}
222130

223-
SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key) const
131+
LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key, size_t level)
224132
{
225-
auto table = get_table();
226-
auto attr = table->get_column_attr(col_key);
227-
REALM_ASSERT(attr.test(col_attr_Set));
228-
bool nullable = attr.test(col_attr_Nullable);
133+
REALM_ASSERT(col_key.get_attrs().test(col_attr_List) || col_key.get_type() == col_type_Mixed);
134+
return create_collection<LstBase, Lst, LnkLst>(col_key, level);
135+
}
229136

230-
switch (table->get_column_type(col_key)) {
231-
case type_Int:
232-
if (nullable)
233-
return std::make_unique<Set<util::Optional<Int>>>(col_key);
234-
return std::make_unique<Set<Int>>(col_key);
235-
case type_Bool:
236-
if (nullable)
237-
return std::make_unique<Set<util::Optional<Bool>>>(col_key);
238-
return std::make_unique<Set<Bool>>(col_key);
239-
case type_Float:
240-
if (nullable)
241-
return std::make_unique<Set<util::Optional<Float>>>(col_key);
242-
return std::make_unique<Set<Float>>(col_key);
243-
case type_Double:
244-
if (nullable)
245-
return std::make_unique<Set<util::Optional<Double>>>(col_key);
246-
return std::make_unique<Set<Double>>(col_key);
247-
case type_String:
248-
return std::make_unique<Set<String>>(col_key);
249-
case type_Binary:
250-
return std::make_unique<Set<Binary>>(col_key);
251-
case type_Timestamp:
252-
return std::make_unique<Set<Timestamp>>(col_key);
253-
case type_Decimal:
254-
return std::make_unique<Set<Decimal128>>(col_key);
255-
case type_ObjectId:
256-
if (nullable)
257-
return std::make_unique<Set<util::Optional<ObjectId>>>(col_key);
258-
return std::make_unique<Set<ObjectId>>(col_key);
259-
case type_UUID:
260-
if (nullable)
261-
return std::make_unique<Set<util::Optional<UUID>>>(col_key);
262-
return std::make_unique<Set<UUID>>(col_key);
263-
case type_TypedLink:
264-
return std::make_unique<Set<ObjLink>>(col_key);
265-
case type_Mixed:
266-
return std::make_unique<Set<Mixed>>(col_key);
267-
case type_Link:
268-
return std::make_unique<LnkSet>(col_key);
269-
}
270-
REALM_TERMINATE("Unsupported column type.");
137+
SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key, size_t level)
138+
{
139+
REALM_ASSERT(col_key.get_attrs().test(col_attr_Set));
140+
return create_collection<SetBase, Set, LnkSet>(col_key, level);
271141
}
272142

273-
CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key) const
143+
CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key, size_t level)
274144
{
275145
if (col_key.is_list()) {
276-
return get_listbase_ptr(col_key);
146+
return get_listbase_ptr(col_key, level);
277147
}
278148
else if (col_key.is_set()) {
279-
return get_setbase_ptr(col_key);
149+
return get_setbase_ptr(col_key, level);
280150
}
281151
else if (col_key.is_dictionary()) {
282-
return std::make_unique<Dictionary>(col_key, get_level() + 1);
152+
return std::make_unique<Dictionary>(col_key, level + 1);
283153
}
284154
return {};
285155
}
286156

287-
288157
int64_t CollectionParent::generate_key(size_t sz)
289158
{
290159
static std::mt19937 gen32;
@@ -307,5 +176,13 @@ int64_t CollectionParent::generate_key(size_t sz)
307176
return key;
308177
}
309178

179+
void CollectionParent::set_key(BPlusTreeMixed& tree, size_t index)
180+
{
181+
int64_t key = generate_key(tree.size());
182+
while (tree.find_key(key) != realm::not_found) {
183+
key++;
184+
}
185+
tree.set_key(index, key);
186+
}
310187

311188
} // namespace realm

‎src/realm/collection_parent.hpp‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,16 @@
2020
#define REALM_COLLECTION_PARENT_HPP
2121

2222
#include <realm/alloc.hpp>
23-
#include <realm/table_ref.hpp>
24-
#include <realm/path.hpp>
2523
#include <realm/mixed.hpp>
24+
#include <realm/path.hpp>
25+
#include <realm/table_ref.hpp>
2626

2727
namespace realm {
2828

2929
class Obj;
3030
class Replication;
3131
class CascadeState;
32+
class BPlusTreeMixed;
3233

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

99+
static LstBasePtr get_listbase_ptr(ColKey col_key, size_t level);
100+
static SetBasePtr get_setbase_ptr(ColKey col_key, size_t level);
101+
static CollectionBasePtr get_collection_ptr(ColKey col_key, size_t level);
102+
103+
static int64_t generate_key(size_t sz);
104+
static void set_key(BPlusTreeMixed& tree, size_t index);
105+
98106
protected:
99107
friend class Collection;
100108
template <class>
@@ -129,20 +137,8 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
129137
/// Set the top ref in parent
130138
virtual void set_collection_ref(Index, ref_type ref, CollectionType) = 0;
131139

140+
/// Get the counter which is incremented whenever the root Obj is updated.
132141
virtual uint32_t parent_version() const noexcept = 0;
133-
134-
// Used when inserting a new link. You will not remove existing links in this process
135-
void set_backlink(ColKey col_key, ObjLink new_link) const;
136-
// Used when replacing a link, return true if CascadeState contains objects to remove
137-
bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const;
138-
// Used when removing a backlink, return true if CascadeState contains objects to remove
139-
bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const;
140-
141-
LstBasePtr get_listbase_ptr(ColKey col_key) const;
142-
SetBasePtr get_setbase_ptr(ColKey col_key) const;
143-
CollectionBasePtr get_collection_ptr(ColKey col_key) const;
144-
145-
static int64_t generate_key(size_t sz);
146142
};
147143

148144
} // namespace realm

‎src/realm/dictionary.cpp‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -441,11 +441,7 @@ void Dictionary::insert_collection(const PathElement& path_elem, CollectionType
441441
if (!old_val || *old_val != new_val) {
442442
m_values->ensure_keys();
443443
auto [it, inserted] = insert(path_elem.get_key(), new_val);
444-
int64_t key = generate_key(size());
445-
while (m_values->find_key(key) != realm::not_found) {
446-
key++;
447-
}
448-
m_values->set_key(it.index(), key);
444+
set_key(*m_values, it.index());
449445
}
450446
}
451447

@@ -682,7 +678,6 @@ void Dictionary::ensure_created()
682678
{
683679
constexpr bool allow_create = true;
684680
if (do_update_if_needed(allow_create) == UpdateStatus::Detached) {
685-
// FIXME: untested
686681
throw StaleAccessor("Dictionary no longer exists");
687682
}
688683
}

0 commit comments

Comments
 (0)