Skip to content

Commit

Permalink
Consolidate collection updating logic somewhat
Browse files Browse the repository at this point in the history
  • Loading branch information
@tgoyne
tgoyne committed Mar 7, 2024
1 parent 5d74b42 commit c911c9f
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 226 deletions.
21 changes: 21 additions & 0 deletions src/realm/collection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,25 @@ void Collection::get_any(QueryCtrlBlock& ctrl, Mixed val, size_t index)
}
}

UpdateStatus CollectionBase::do_init_from_parent(BPlusTreeBase* tree, ref_type ref, bool allow_create)
{
if (ref) {
tree->init_from_ref(ref);
}
else {
if (tree->init_from_parent()) {
// All is well
return UpdateStatus::Updated;
}
if (!allow_create) {
tree->detach();
return UpdateStatus::Detached;
}
// The ref in the column was NULL, create the tree in place.
tree->create();
REALM_ASSERT(tree->is_attached());
}
return UpdateStatus::Updated;
}

} // namespace realm
46 changes: 21 additions & 25 deletions src/realm/collection.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,18 @@ class DummyParent : public CollectionParent {
{
return m_obj;
}
uint32_t parent_version() const noexcept final
{
return 0;
}

protected:
Obj m_obj;
ref_type m_ref;
UpdateStatus update_if_needed_with_status() const final
UpdateStatus update_if_needed() const final
{
return UpdateStatus::Updated;
}
bool update_if_needed() const final
{
return true;
}
ref_type get_collection_ref(Index, CollectionType) const final
{
return m_ref;
Expand Down Expand Up @@ -255,6 +255,7 @@ class CollectionBase : public Collection {
CollectionBase& operator=(CollectionBase&&) noexcept = default;

void validate_index(const char* msg, size_t index, size_t size) const;
static UpdateStatus do_init_from_parent(BPlusTreeBase* tree, ref_type ref, bool allow_create);
};

inline std::string_view collection_type_name(CollectionType col_type, bool uppercase = false)
Expand Down Expand Up @@ -492,7 +493,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
if (m_parent) {
try {
// Update the parent. Will throw if parent is not existing.
switch (m_parent->update_if_needed_with_status()) {
switch (m_parent->update_if_needed()) {
case UpdateStatus::Updated:
// Make sure to update next time around
m_content_version = 0;
Expand Down Expand Up @@ -524,7 +525,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
{
try {
// `has_changed()` sneakily modifies internal state.
update_if_needed_with_status();
update_if_needed();
if (m_last_content_version != m_content_version) {
m_last_content_version = m_content_version;
return true;
Expand Down Expand Up @@ -573,14 +574,12 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
Obj m_obj_mem;
std::shared_ptr<CollectionParent> m_col_parent;
CollectionParent::Index m_index;
mutable size_t m_my_version = 0;
ColKey m_col_key;
bool m_nullable = false;

mutable uint_fast64_t m_content_version = 0;

// Content version used by `has_changed()`.
mutable uint_fast64_t m_last_content_version = 0;
mutable uint32_t m_parent_version = 0;
bool m_nullable = false;

CollectionBaseImpl() = default;
CollectionBaseImpl(const CollectionBaseImpl& other)
Expand Down Expand Up @@ -650,13 +649,14 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {

UpdateStatus get_update_status() const
{
UpdateStatus status = m_parent ? m_parent->update_if_needed_with_status() : UpdateStatus::Detached;
UpdateStatus status = m_parent ? m_parent->update_if_needed() : UpdateStatus::Detached;

if (status != UpdateStatus::Detached) {
auto content_version = m_alloc->get_content_version();
if (content_version != m_content_version || m_my_version != m_parent->m_parent_version) {
auto parent_version = m_parent->parent_version();
if (content_version != m_content_version || m_parent_version != parent_version) {
m_content_version = content_version;
m_my_version = m_parent->m_parent_version;
m_parent_version = parent_version;
status = UpdateStatus::Updated;
}
}
Expand All @@ -667,18 +667,14 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
/// Refresh the parent object (if needed) and compare version numbers.
/// Return true if the collection should initialize from parent
/// Throws if the owning object no longer exists.
bool should_update()
bool should_update() const
{
check_parent();
bool changed = m_parent->update_if_needed(); // Throws if the object does not exist.
auto content_version = m_alloc->get_content_version();

if (changed || content_version != m_content_version || m_my_version != m_parent->m_parent_version) {
m_content_version = content_version;
m_my_version = m_parent->m_parent_version;
return true;
auto status = get_update_status();
if (status == UpdateStatus::Detached) {
throw StaleAccessor("Parent no longer exists");
}
return false;
return status == UpdateStatus::Updated;
}

void bump_content_version()
Expand Down Expand Up @@ -796,7 +792,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
///
/// If no change has happened to the data, this function returns
/// `UpdateStatus::NoChange`, and the caller is allowed to not do anything.
virtual UpdateStatus update_if_needed_with_status() const = 0;
virtual UpdateStatus update_if_needed() const = 0;
};

namespace _impl {
Expand Down Expand Up @@ -884,7 +880,7 @@ class ObjCollectionBase : public Interface, public _impl::ObjListProxy {
/// `BPlusTree<T>`.
virtual BPlusTree<ObjKey>* get_mutable_tree() const = 0;

/// Implements update_if_needed() in a way that ensures the consistency of
/// Implements `update_if_needed()` in a way that ensures the consistency of
/// the unresolved list. Derived classes should call this instead of calling
/// `update_if_needed()` on their inner accessor.
UpdateStatus update_if_needed() const
Expand Down
14 changes: 6 additions & 8 deletions src/realm/collection_parent.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
using Index = StableIndex;

// Return the nesting level of the parent
size_t get_level() const noexcept
uint8_t get_level() const noexcept
{
return m_level;
}
Expand Down Expand Up @@ -106,21 +106,17 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
#else
static constexpr size_t s_max_level = 100;
#endif
size_t m_level = 0;
mutable size_t m_parent_version = 0;
uint8_t m_level = 0;

constexpr CollectionParent(size_t level = 0)
constexpr CollectionParent(uint8_t level = 0)
: m_level(level)
{
}

virtual ~CollectionParent();
/// Update the accessor (and return `UpdateStatus::Detached` if the
// collection is not initialized.
virtual UpdateStatus update_if_needed_with_status() const = 0;
/// Check if the storage version has changed and update if it has
/// Return true if the object was updated
virtual bool update_if_needed() const = 0;
virtual UpdateStatus update_if_needed() const = 0;
/// Get owning object
virtual const Obj& get_object() const noexcept = 0;
/// Get the top ref from pareht
Expand All @@ -133,6 +129,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;

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
Expand Down
49 changes: 18 additions & 31 deletions src/realm/dictionary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,10 +651,9 @@ size_t Dictionary::find_index(const Index& index) const
return m_values->find_key(index.get_salt());
}

UpdateStatus Dictionary::update_if_needed_with_status() const
UpdateStatus Dictionary::do_update_if_needed(bool allow_create) const
{
auto status = Base::get_update_status();
switch (status) {
switch (get_update_status()) {
case UpdateStatus::Detached: {
m_dictionary_top.reset();
return UpdateStatus::Detached;
Expand All @@ -667,27 +666,24 @@ UpdateStatus Dictionary::update_if_needed_with_status() const
// perform lazy initialization by treating it as an update.
[[fallthrough]];
}
case UpdateStatus::Updated: {
// Try to initialize. If the dictionary is not initialized
// the function will return false;
bool attached = init_from_parent(false);
Base::update_content_version();
CollectionParent::m_parent_version++;
return attached ? UpdateStatus::Updated : UpdateStatus::Detached;
}
case UpdateStatus::Updated:
return init_from_parent(allow_create);
}
REALM_UNREACHABLE();
}

UpdateStatus Dictionary::update_if_needed() const
{
constexpr bool allow_create = false;
return do_update_if_needed(allow_create);
}

void Dictionary::ensure_created()
{
if (Base::should_update() || !(m_dictionary_top && m_dictionary_top->is_attached())) {
// When allow_create is true, init_from_parent will always succeed
// In case of errors, an exception is thrown.
constexpr bool allow_create = true;
init_from_parent(allow_create); // Throws
CollectionParent::m_parent_version++;
Base::update_content_version();
constexpr bool allow_create = true;
if (do_update_if_needed(allow_create) == UpdateStatus::Detached) {
// FIXME: untested
throw StaleAccessor("Dictionary no longer exists");
}
}

Expand Down Expand Up @@ -837,8 +833,9 @@ void Dictionary::clear()
}
}

bool Dictionary::init_from_parent(bool allow_create) const
UpdateStatus Dictionary::init_from_parent(bool allow_create) const
{
Base::update_content_version();
try {
auto ref = Base::get_collection_ref();
if ((ref || allow_create) && !m_dictionary_top) {
Expand Down Expand Up @@ -871,7 +868,7 @@ bool Dictionary::init_from_parent(bool allow_create) const
// dictionary detached
if (!allow_create) {
m_dictionary_top.reset();
return false;
return UpdateStatus::Detached;
}

// Create dictionary
Expand All @@ -881,7 +878,7 @@ bool Dictionary::init_from_parent(bool allow_create) const
m_dictionary_top->update_parent();
}

return true;
return UpdateStatus::Updated;
}
catch (...) {
m_dictionary_top.reset();
Expand Down Expand Up @@ -1179,7 +1176,6 @@ ref_type Dictionary::get_collection_ref(Index index, CollectionType type) const
throw realm::IllegalOperation(util::format("Not a %1", type));
}
throw StaleAccessor("This collection is no more");
return 0;
}

bool Dictionary::check_collection_ref(Index index, CollectionType type) const noexcept
Expand All @@ -1200,15 +1196,6 @@ void Dictionary::set_collection_ref(Index index, ref_type ref, CollectionType ty
m_values->set(ndx, Mixed(ref, type));
}

bool Dictionary::update_if_needed() const
{
auto status = update_if_needed_with_status();
if (status == UpdateStatus::Detached) {
throw StaleAccessor("CollectionList no longer exists");
}
return status == UpdateStatus::Updated;
}

/************************* DictionaryLinkValues *************************/

DictionaryLinkValues::DictionaryLinkValues(const Obj& obj, ColKey col_key)
Expand Down
22 changes: 12 additions & 10 deletions src/realm/dictionary.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
using Base = CollectionBaseImpl<DictionaryBase>;
class Iterator;

Dictionary()
: CollectionParent(0)
{
}
Dictionary() = default;
~Dictionary();

Dictionary(const Obj& obj, ColKey col_key)
Expand Down Expand Up @@ -213,12 +210,15 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
{
return get_obj().get_table();
}
UpdateStatus update_if_needed_with_status() const override;
bool update_if_needed() const override;
UpdateStatus update_if_needed() const override;
const Obj& get_object() const noexcept override
{
return get_obj();
}
uint32_t parent_version() const noexcept override
{
return m_parent_version;
}
ref_type get_collection_ref(Index, CollectionType) const override;
bool check_collection_ref(Index, CollectionType) const noexcept override;
void set_collection_ref(Index, ref_type ref, CollectionType) override;
Expand All @@ -239,7 +239,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle

Dictionary(Allocator& alloc, ColKey col_key, ref_type ref);

bool init_from_parent(bool allow_create) const;
UpdateStatus init_from_parent(bool allow_create) const;
Mixed do_get(size_t ndx) const;
void do_erase(size_t ndx, Mixed key);
Mixed do_get_key(size_t ndx) const;
Expand All @@ -261,12 +261,14 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
void do_accumulate(size_t* return_ndx, AggregateType& agg) const;

void ensure_created();
inline bool update() const
bool update() const
{
return update_if_needed_with_status() != UpdateStatus::Detached;
return update_if_needed() != UpdateStatus::Detached;
}
void verify() const;
void get_key_type();

UpdateStatus do_update_if_needed(bool allow_create) const;
};

class Dictionary::Iterator {
Expand Down Expand Up @@ -459,7 +461,7 @@ class DictionaryLinkValues final : public ObjCollectionBase<CollectionBase> {
// Overrides of ObjCollectionBase:
UpdateStatus do_update_if_needed() const final
{
return m_source.update_if_needed_with_status();
return m_source.update_if_needed();
}
BPlusTree<ObjKey>* get_mutable_tree() const final
{
Expand Down
Loading

0 comments on commit c911c9f

Please sign in to comment.