Skip to content

Commit 08546b7

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 1792e59 commit 08546b7

19 files changed

+384
-293
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* Fixed conflict resolution bug which may result in an crash when the AddInteger instruction on Mixed properties is merged against updates to a non-integer type ([PR #7353](https://github.com/realm/realm-core/pull/7353)).
99
* Fix a spurious crash related to opening a Realm on background thread while the process was in the middle of exiting ([#7420](https://github.com/realm/realm-core/issues/7420jj))
1010
* 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).
11+
* 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).
1112

1213
### Breaking changes
1314
* Remove `realm_scheduler_set_default_factory()` and `realm_scheduler_has_default_factory()`, and change the `Scheduler` factory function to a bare function pointer rather than a `UniqueFunction` so that it does not have a non-trivial destructor.

‎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)