Improve change calculation performance for nontrivial object graphs

Skip doing any checking at all if none of the tables reachable from the root
table have been modified (which can happen if the table version was bumped due
to insertions, unrelated backlinks, or unlinked-to rows being deleted in linked
tables).

Add cycle checking rather than relying on the max depth to handle it, as the
worst case was O(N^16) if the cycle involved a LinkList of size N.

Track which rows have been confirmed to have not been modified.

Cache the information about the links for each of the relevant tables as
checking the table schema can get somewhat expensive.
This commit is contained in:
Thomas Goyne 2016-04-29 10:47:54 -07:00
parent f9364b50a4
commit d8a69b87dc
5 changed files with 355 additions and 194 deletions

View File

@ -26,31 +26,87 @@
using namespace realm;
using namespace realm::_impl;
bool TransactionChangeInfo::row_did_change(Table const& table, size_t idx, int depth) const
std::function<bool (size_t)>
CollectionNotifier::get_modification_checker(TransactionChangeInfo const& info,
Table const& root_table)
{
if (depth > 16) // arbitrary limit
// First check if any of the tables accessible from the root table were
// actually modified. This can be false if there were only insertions, or
// deletions which were not linked to by any row in the linking table
auto table_modified = [&](auto& tbl) {
return tbl.table_ndx < info.tables.size()
&& !info.tables[tbl.table_ndx].modifications.empty();
};
if (!any_of(begin(m_related_tables), end(m_related_tables), table_modified)) {
return [](size_t) { return false; };
}
return DeepChangeChecker(info, root_table, m_related_tables);
}
void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& out, Table const& table)
{
auto table_ndx = table.get_index_in_group();
if (any_of(begin(out), end(out), [=](auto& tbl) { return tbl.table_ndx == table_ndx; }))
return;
size_t info = out.size();
out.push_back({table_ndx, {}});
for (size_t i = 0, count = table.get_column_count(); i != count; ++i) {
auto type = table.get_column_type(i);
if (type == type_Link || type == type_LinkList) {
out[info].links.push_back({i, type == type_LinkList});
find_related_tables(out, *table.get_link_target(i));
}
}
}
DeepChangeChecker::DeepChangeChecker(TransactionChangeInfo const& info,
Table const& root_table,
std::vector<RelatedTable> const& related_tables)
: m_info(info)
, m_root_table(root_table)
, m_root_table_ndx(root_table.get_index_in_group())
, m_root_modifications(m_root_table_ndx < info.tables.size() ? &info.tables[m_root_table_ndx].modifications : nullptr)
, m_related_tables(related_tables)
{
}
bool DeepChangeChecker::check_outgoing_links(size_t table_ndx,
Table const& table,
size_t row_ndx, size_t depth)
{
auto it = find_if(begin(m_related_tables), end(m_related_tables),
[&](auto&& tbl) { return tbl.table_ndx == table_ndx; });
if (it == m_related_tables.end())
return false;
size_t table_ndx = table.get_index_in_group();
if (table_ndx < tables.size() && tables[table_ndx].modifications.contains(idx))
return true;
for (size_t i = 0, count = table.get_column_count(); i < count; ++i) {
auto type = table.get_column_type(i);
if (type == type_Link) {
if (table.is_null_link(i, idx))
continue;
auto dst = table.get_link(i, idx);
return row_did_change(*table.get_link_target(i), dst, depth + 1);
// Check if we're already checking if the destination of the link is
// modified, and if not add it to the stack
auto already_checking = [&](size_t col) {
for (auto p = m_current_path.begin(); p < m_current_path.begin() + depth; ++p) {
if (p->table == table_ndx && p->row == row_ndx && p->col == col)
return true;
}
if (type != type_LinkList)
continue;
m_current_path[depth] = {table_ndx, row_ndx, col, false};
return false;
};
auto& target = *table.get_link_target(i);
auto lvr = table.get_linklist(i, idx);
for (size_t j = 0; j < lvr->size(); ++j) {
for (auto const& link : it->links) {
if (already_checking(link.col_ndx))
continue;
if (!link.is_list) {
if (table.is_null_link(link.col_ndx, row_ndx))
continue;
auto dst = table.get_link(link.col_ndx, row_ndx);
return check_row(*table.get_link_target(link.col_ndx), dst, depth + 1);
}
auto& target = *table.get_link_target(link.col_ndx);
auto lvr = table.get_linklist(link.col_ndx, row_ndx);
for (size_t j = 0, size = lvr->size(); j < size; ++j) {
size_t dst = lvr->get(j).get_index();
if (row_did_change(target, dst, depth + 1))
if (check_row(target, dst, depth + 1))
return true;
}
}
@ -58,6 +114,39 @@ bool TransactionChangeInfo::row_did_change(Table const& table, size_t idx, int d
return false;
}
bool DeepChangeChecker::check_row(Table const& table, size_t idx, size_t depth)
{
// Arbitrary upper limit on the maximum depth to search
if (depth >= m_current_path.size()) {
// Don't mark any of the intermediate rows checked along the path as
// not modified, as a search starting from them might hit a modification
for (size_t i = 1; i < m_current_path.size(); ++i)
m_current_path[i].depth_exceeded = true;
return false;
}
size_t table_ndx = table.get_index_in_group();
if (depth > 0 && table_ndx < m_info.tables.size() && m_info.tables[table_ndx].modifications.contains(idx))
return true;
if (m_not_modified.size() <= table_ndx)
m_not_modified.resize(table_ndx + 1);
if (m_not_modified[table_ndx].contains(idx))
return false;
bool ret = check_outgoing_links(table_ndx, table, idx, depth);
if (!ret && !m_current_path[depth].depth_exceeded)
m_not_modified[table_ndx].add(idx);
return ret;
}
bool DeepChangeChecker::operator()(size_t ndx)
{
if (m_root_modifications && m_root_modifications->contains(ndx))
return true;
return check_row(m_root_table, ndx, 0);
}
CollectionNotifier::CollectionNotifier(std::shared_ptr<Realm> realm)
: m_realm(std::move(realm))
, m_sg_version(Realm::Internal::get_shared_group(*m_realm).get_version_of_current_transaction())
@ -139,24 +228,10 @@ std::unique_lock<std::mutex> CollectionNotifier::lock_target()
return std::unique_lock<std::mutex>{m_realm_mutex};
}
// Recursively add `table` and all tables it links to to `out`
static void find_relevant_tables(std::vector<size_t>& out, Table const& table)
{
auto table_ndx = table.get_index_in_group();
if (find(begin(out), end(out), table_ndx) != end(out))
return;
out.push_back(table_ndx);
for (size_t i = 0, count = table.get_column_count(); i != count; ++i) {
if (table.get_column_type(i) == type_Link || table.get_column_type(i) == type_LinkList) {
find_relevant_tables(out, *table.get_link_target(i));
}
}
}
void CollectionNotifier::set_table(Table const& table)
{
find_relevant_tables(m_relevant_tables, table);
m_related_tables.clear();
DeepChangeChecker::find_related_tables(m_related_tables, table);
}
void CollectionNotifier::add_required_change_info(TransactionChangeInfo& info)
@ -165,11 +240,13 @@ void CollectionNotifier::add_required_change_info(TransactionChangeInfo& info)
return;
}
auto max = *max_element(begin(m_relevant_tables), end(m_relevant_tables)) + 1;
if (max > info.table_modifications_needed.size())
info.table_modifications_needed.resize(max, false);
for (auto table_ndx : m_relevant_tables) {
info.table_modifications_needed[table_ndx] = true;
auto max = max_element(begin(m_related_tables), end(m_related_tables),
[](auto&& a, auto&& b) { return a.table_ndx < b.table_ndx; });
if (max->table_ndx >= info.table_modifications_needed.size())
info.table_modifications_needed.resize(max->table_ndx + 1, false);
for (auto& tbl : m_related_tables) {
info.table_modifications_needed[tbl.table_ndx] = true;
}
}

View File

@ -23,6 +23,7 @@
#include <realm/group_shared.hpp>
#include <array>
#include <atomic>
#include <exception>
#include <functional>
@ -45,8 +46,47 @@ struct TransactionChangeInfo {
std::vector<bool> table_moves_needed;
std::vector<ListChangeInfo> lists;
std::vector<CollectionChangeBuilder> tables;
};
bool row_did_change(Table const& table, size_t row_ndx, int depth = 0) const;
class DeepChangeChecker {
public:
struct OutgoingLink {
size_t col_ndx;
bool is_list;
};
struct RelatedTable {
size_t table_ndx;
std::vector<OutgoingLink> links;
};
DeepChangeChecker(TransactionChangeInfo const& info, Table const& root_table,
std::vector<RelatedTable> const& related_tables);
bool operator()(size_t row_ndx);
// Recursively add `table` and all tables it links to to `out`, along with
// information about the links from them
static void find_related_tables(std::vector<RelatedTable>& out, Table const& table);
private:
TransactionChangeInfo const& m_info;
Table const& m_root_table;
const size_t m_root_table_ndx;
IndexSet const* const m_root_modifications;
std::vector<IndexSet> m_not_modified;
std::vector<RelatedTable> const& m_related_tables;
struct Path {
size_t table;
size_t row;
size_t col;
bool depth_exceeded;
};
std::array<Path, 16> m_current_path;
bool check_row(Table const& table, size_t row_ndx, size_t depth = 0);
bool check_outgoing_links(size_t table_ndx, Table const& table,
size_t row_ndx, size_t depth = 0);
};
// A base class for a notifier that keeps a collection up to date and/or
@ -116,6 +156,8 @@ protected:
void set_table(Table const& table);
std::unique_lock<std::mutex> lock_target();
std::function<bool (size_t)> get_modification_checker(TransactionChangeInfo const&, Table const&);
private:
virtual void do_attach_to(SharedGroup&) = 0;
virtual void do_detach_from(SharedGroup&) = 0;
@ -133,8 +175,7 @@ private:
CollectionChangeBuilder m_accumulated_changes;
CollectionChangeSet m_changes_to_deliver;
// Tables which this collection needs change information for
std::vector<size_t> m_relevant_tables;
std::vector<DeepChangeChecker::RelatedTable> m_related_tables;
struct Callback {
CollectionChangeCallback fn;

View File

@ -98,17 +98,18 @@ void ListNotifier::run()
return;
}
auto row_did_change = get_modification_checker(*m_info, m_lv->get_target_table());
for (size_t i = 0; i < m_lv->size(); ++i) {
if (m_change.modifications.contains(i))
continue;
if (m_info->row_did_change(m_lv->get_target_table(), m_lv->get(i).get_index()))
if (row_did_change(m_lv->get(i).get_index()))
m_change.modifications.add(i);
}
for (auto const& move : m_change.moves) {
if (m_change.modifications.contains(move.to))
continue;
if (m_info->row_did_change(m_lv->get_target_table(), m_lv->get(move.to).get_index()))
if (row_did_change(m_lv->get(move.to).get_index()))
m_change.modifications.add(move.to);
}

View File

@ -124,7 +124,7 @@ void ResultsNotifier::calculate_changes()
}
m_changes = CollectionChangeBuilder::calculate(m_previous_rows, next_rows,
[&](size_t row) { return m_info->row_did_change(*m_query->get_table(), row); },
get_modification_checker(*m_info, *m_query->get_table()),
m_sort || m_from_linkview);
m_previous_rows = std::move(next_rows);

View File

@ -177,154 +177,6 @@ TEST_CASE("Transaction log parsing") {
}
}
SECTION("row_did_change()") {
config.schema = std::make_unique<Schema>(Schema{
{"table", "", {
{"int", PropertyTypeInt},
{"link", PropertyTypeObject, "table", false, false, true},
{"array", PropertyTypeArray, "table"}
}},
});
auto r = Realm::get_shared_realm(config);
auto table = r->read_group()->get_table("class_table");
r->begin_transaction();
table->add_empty_row(10);
for (int i = 0; i < 10; ++i)
table->set_int(0, i, i);
r->commit_transaction();
auto track_changes = [&](auto&& f) {
auto history = make_client_history(config.path);
SharedGroup sg(*history, SharedGroup::durability_MemOnly);
Group const& g = sg.begin_read();
r->begin_transaction();
f();
r->commit_transaction();
_impl::TransactionChangeInfo info;
info.table_modifications_needed.resize(g.size(), true);
info.table_moves_needed.resize(g.size(), true);
_impl::transaction::advance(sg, info);
return info;
};
SECTION("direct changes are tracked") {
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE_FALSE(info.row_did_change(*table, 8));
REQUIRE(info.row_did_change(*table, 9));
}
SECTION("changes over links are tracked") {
r->begin_transaction();
for (int i = 0; i < 9; ++i)
table->set_link(1, i, i + 1);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE(info.row_did_change(*table, 0));
}
SECTION("changes over linklists are tracked") {
r->begin_transaction();
for (int i = 0; i < 9; ++i)
table->get_linklist(2, i)->add(i + 1);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE(info.row_did_change(*table, 0));
}
SECTION("cycles over links do not loop forever") {
r->begin_transaction();
table->set_link(1, 0, 0);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE_FALSE(info.row_did_change(*table, 0));
}
SECTION("cycles over linklists do not loop forever") {
r->begin_transaction();
table->get_linklist(2, 0)->add(0);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE_FALSE(info.row_did_change(*table, 0));
}
SECTION("targets moving is not a change") {
r->begin_transaction();
table->set_link(1, 0, 9);
table->get_linklist(2, 0)->add(9);
r->commit_transaction();
auto info = track_changes([&] {
table->move_last_over(5);
});
REQUIRE_FALSE(info.row_did_change(*table, 0));
}
SECTION("changes made before a row is moved are reported") {
r->begin_transaction();
table->set_link(1, 0, 9);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 5);
table->move_last_over(5);
});
REQUIRE(info.row_did_change(*table, 0));
r->begin_transaction();
table->get_linklist(2, 0)->add(8);
r->commit_transaction();
info = track_changes([&] {
table->set_int(0, 8, 5);
table->move_last_over(5);
});
REQUIRE(info.row_did_change(*table, 0));
}
SECTION("changes made after a row is moved are reported") {
r->begin_transaction();
table->set_link(1, 0, 9);
r->commit_transaction();
auto info = track_changes([&] {
table->move_last_over(5);
table->set_int(0, 5, 5);
});
REQUIRE(info.row_did_change(*table, 0));
r->begin_transaction();
table->get_linklist(2, 0)->add(8);
r->commit_transaction();
info = track_changes([&] {
table->move_last_over(5);
table->set_int(0, 5, 5);
});
REQUIRE(info.row_did_change(*table, 0));
}
}
SECTION("table change information") {
config.schema = std::make_unique<Schema>(Schema{
{"table", "", {
@ -947,3 +799,193 @@ TEST_CASE("Transaction log parsing") {
}
}
}
TEST_CASE("DeepChangeChecker") {
InMemoryTestFile config;
config.automatic_change_notifications = false;
config.schema = std::make_unique<Schema>(Schema{
{"table", "", {
{"int", PropertyTypeInt},
{"link", PropertyTypeObject, "table", false, false, true},
{"array", PropertyTypeArray, "table"}
}},
});
auto r = Realm::get_shared_realm(config);
auto table = r->read_group()->get_table("class_table");
r->begin_transaction();
table->add_empty_row(10);
for (int i = 0; i < 10; ++i)
table->set_int(0, i, i);
r->commit_transaction();
auto track_changes = [&](auto&& f) {
auto history = make_client_history(config.path);
SharedGroup sg(*history, SharedGroup::durability_MemOnly);
Group const& g = sg.begin_read();
r->begin_transaction();
f();
r->commit_transaction();
_impl::TransactionChangeInfo info;
info.table_modifications_needed.resize(g.size(), true);
info.table_moves_needed.resize(g.size(), true);
_impl::transaction::advance(sg, info);
return info;
};
std::vector<_impl::DeepChangeChecker::RelatedTable> tables;
_impl::DeepChangeChecker::find_related_tables(tables, *table);
SECTION("direct changes are tracked") {
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
_impl::DeepChangeChecker checker(info, *table, tables);
REQUIRE_FALSE(checker(8));
REQUIRE(checker(9));
}
SECTION("changes over links are tracked") {
r->begin_transaction();
for (int i = 0; i < 9; ++i)
table->set_link(1, i, i + 1);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("changes over linklists are tracked") {
r->begin_transaction();
for (int i = 0; i < 9; ++i)
table->get_linklist(2, i)->add(i + 1);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("cycles over links do not loop forever") {
r->begin_transaction();
table->set_link(1, 0, 0);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("cycles over linklists do not loop forever") {
r->begin_transaction();
table->get_linklist(2, 0)->add(0);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 10);
});
REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("link chains are tracked up to 16 levels deep") {
r->begin_transaction();
table->add_empty_row(10);
for (int i = 0; i < 19; ++i)
table->set_link(1, i, i + 1);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 19, -1);
});
_impl::DeepChangeChecker checker(info, *table, tables);
CHECK(checker(19));
CHECK(checker(18));
CHECK(checker(4));
CHECK_FALSE(checker(3));
CHECK_FALSE(checker(2));
// Check in other orders to make sure that the caching doesn't effect
// the results
_impl::DeepChangeChecker checker2(info, *table, tables);
CHECK_FALSE(checker2(2));
CHECK_FALSE(checker2(3));
CHECK(checker2(4));
CHECK(checker2(18));
CHECK(checker2(19));
_impl::DeepChangeChecker checker3(info, *table, tables);
CHECK(checker2(4));
CHECK_FALSE(checker2(3));
CHECK_FALSE(checker2(2));
CHECK(checker2(18));
CHECK(checker2(19));
}
SECTION("targets moving is not a change") {
r->begin_transaction();
table->set_link(1, 0, 9);
table->get_linklist(2, 0)->add(9);
r->commit_transaction();
auto info = track_changes([&] {
table->move_last_over(5);
});
REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("changes made before a row is moved are reported") {
r->begin_transaction();
table->set_link(1, 0, 9);
r->commit_transaction();
auto info = track_changes([&] {
table->set_int(0, 9, 5);
table->move_last_over(5);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
r->begin_transaction();
table->get_linklist(2, 0)->add(8);
r->commit_transaction();
info = track_changes([&] {
table->set_int(0, 8, 5);
table->move_last_over(5);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
SECTION("changes made after a row is moved are reported") {
r->begin_transaction();
table->set_link(1, 0, 9);
r->commit_transaction();
auto info = track_changes([&] {
table->move_last_over(5);
table->set_int(0, 5, 5);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
r->begin_transaction();
table->get_linklist(2, 0)->add(8);
r->commit_transaction();
info = track_changes([&] {
table->move_last_over(5);
table->set_int(0, 5, 5);
});
REQUIRE(_impl::DeepChangeChecker(info, *table, tables)(0));
}
}