Skip to content

Commit

Permalink
Remove support for sorting TableViews which aren't up to date
Browse files Browse the repository at this point in the history
This was never actually used (Results().snapshot().sort() produces a live
Results) and had a significant performance impact.
  • Loading branch information
tgoyne committed May 24, 2023
1 parent 6b4f275 commit 3467913
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 14 deletions.
24 changes: 10 additions & 14 deletions src/realm/table_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,15 @@ void TableView::sort(ColKey column, bool ascending)
// Sort according to multiple columns, user specified order on each column
void TableView::sort(SortDescriptor order)
{
REALM_ASSERT(is_in_sync());
m_descriptor_ordering.append_sort(std::move(order), SortDescriptor::MergeMode::prepend);
m_descriptor_ordering.collect_dependencies(m_table.unchecked_ptr());

// Adding the new sort descriptor marked us as no longer in sync, but do_sort()
// will bring us back into sync
m_last_seen_versions.clear();
get_dependencies(m_last_seen_versions);

do_sort(m_descriptor_ordering);
}

Expand All @@ -455,6 +461,7 @@ void TableView::do_sync()
// - Table::get_backlink_view()
// Here we sync with the respective source.
m_last_seen_versions.clear();
get_dependencies(m_last_seen_versions);

if (m_collection_source) {
m_key_values.clear();
Expand Down Expand Up @@ -493,8 +500,6 @@ void TableView::do_sync()
}

do_sort(m_descriptor_ordering);

get_dependencies(m_last_seen_versions);
}

void TableView::do_sort(const DescriptorOrdering& ordering)
Expand All @@ -504,22 +509,15 @@ void TableView::do_sort(const DescriptorOrdering& ordering)
size_t sz = size();
if (sz == 0)
return;
REALM_ASSERT(is_in_sync());

// Gather the current rows into a container we can use std algorithms on
size_t detached_ref_count = 0;
BaseDescriptor::IndexPairs index_pairs;
index_pairs.reserve(sz);
// always put any detached refs at the end of the sort
// FIXME: reconsider if this is the right thing to do
// FIXME: consider specialized implementations in derived classes
// (handling detached refs is not required in linkviews)
for (size_t t = 0; t < sz; t++) {
ObjKey key = get_key(t);
if (m_table->is_valid(key)) {
index_pairs.emplace_back(key, t);
}
else
++detached_ref_count;
REALM_ASSERT_DEBUG(m_table->is_valid(key));
index_pairs.emplace_back(key, t);
}

const int num_descriptors = int(ordering.size());
Expand All @@ -541,8 +539,6 @@ void TableView::do_sort(const DescriptorOrdering& ordering)
for (auto& pair : index_pairs) {
m_key_values.add(pair.key_for_object);
}
for (size_t t = 0; t < detached_ref_count; ++t)
m_key_values.add(null_key);
}

bool TableView::is_in_table_order() const
Expand Down
23 changes: 23 additions & 0 deletions test/object-store/results.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3638,6 +3638,29 @@ TEST_CASE("results: snapshots") {
REQUIRE_FALSE(snapshot.first()->is_valid());
REQUIRE_FALSE(snapshot.last()->is_valid());
}

SECTION("sorting a snapshot results in live Results") {
auto table = r->read_group().get_table("class_object");
write([=] {
table->create_object();
});

auto snapshot = Results(r, table).snapshot();
auto sorted = snapshot.sort({{"value", true}});
REQUIRE(snapshot.size() == 1);
REQUIRE(sorted.size() == 1);

write([=] {
table->clear();
});

REQUIRE(snapshot.size() == 1);
REQUIRE(sorted.size() == 0);

// Create a second sorted Results derived from the now-stale snapshot
sorted = snapshot.sort({{"value", true}});
REQUIRE(sorted.size() == 0);
}
}

TEST_CASE("results: distinct") {
Expand Down

0 comments on commit 3467913

Please sign in to comment.