From 49b5f47e843f12e644d6c8f337dcebd3c9b444bb Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 12 Jan 2022 22:08:51 -0800 Subject: [PATCH] Greatly improve performance of sorting dictionaries Dictionary lookup by index is slow, so read all of the keys/values from the dictionary into a vector and sort that instead. Using a dictionary iterator to read the values replaces O(N log N) ClusterTree lookups with O(log N), so this is asymptotically faster. The benchmark is sped up from 3.77 seconds to 21.62 milliseconds, but more reasonably sized Dictionaries will see proportionally smaller benefits. The downside of this is that the array of Mixed requires 24 bytes of scratch space per element in the dictionary. We already require 8 bytes per element to store the results, so this is just a constant factor increase rather than an aysmtotic change in memory use and it's probably not a problem. --- CHANGELOG.md | 3 +- src/realm/dictionary.cpp | 74 ++++++++++++++++++---------- src/realm/dictionary.hpp | 3 ++ test/benchmark-common-tasks/main.cpp | 2 +- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0e6f71da13..e3c20bb637b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) -* Support exporting data from a local realm to a synchonized realm. ([#5018](https://github.com/realm/realm-core/issues/5018)) +* Support exporting data from a local realm to a synchronized realm. ([#5018](https://github.com/realm/realm-core/issues/5018)) +* Greatly improve the performance of sorting or distincting a Dictionary's keys or values. The most expensive operation is now performed O(log N) rather than O(N log N) times, and large Dictionaries can see upwards of 99% reduction in time to sort. ([PR #5166](https://github.com/realm/realm-core/pulls/5166)) ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index c9302c5386c..f6698165940 100644 --- a/src/realm/dictionary.cpp +++ b/src/realm/dictionary.cpp @@ -451,27 +451,49 @@ void Dictionary::align_indices(std::vector& indices) const } } -void Dictionary::sort(std::vector& indices, bool ascending) const +namespace { +std::vector get_keys(const Dictionary& dict) +{ + std::vector values; + values.reserve(dict.size()); + for (auto it = dict.begin(), end = dict.end(); it != end; ++it) + values.push_back(it.key()); + return values; +} + +std::vector get_values(const Dictionary& dict) +{ + std::vector values; + values.reserve(dict.size()); + for (auto it = dict.begin(), end = dict.end(); it != end; ++it) + values.push_back(it.value()); + return values; +} + +void do_sort(std::vector& indices, bool ascending, const std::vector& values) { - align_indices(indices); auto b = indices.begin(); auto e = indices.end(); - std::sort(b, e, [this, ascending](size_t i1, size_t i2) { - auto v1 = get_any(i1); - auto v2 = get_any(i2); - return ascending ? v1 < v2 : v2 < v1; + std::sort(b, e, [ascending, &values](size_t i1, size_t i2) { + return ascending ? values[i1] < values[i2] : values[i2] < values[i1]; }); } +} // anonymous namespace -void Dictionary::distinct(std::vector& indices, util::Optional ascending) const +void Dictionary::sort(std::vector& indices, bool ascending) const { align_indices(indices); + do_sort(indices, ascending, get_values(*this)); +} - bool sort_ascending = ascending ? *ascending : true; - sort(indices, sort_ascending); +void Dictionary::distinct(std::vector& indices, util::Optional ascending) const +{ + align_indices(indices); + auto values = get_values(*this); + do_sort(indices, ascending.value_or(true), values); indices.erase(std::unique(indices.begin(), indices.end(), - [this](size_t i1, size_t i2) { - return get_any(i1) == get_any(i2); + [&values](size_t i1, size_t i2) { + return values[i1] == values[i2]; }), indices.end()); @@ -484,13 +506,7 @@ void Dictionary::distinct(std::vector& indices, util::Optional asc void Dictionary::sort_keys(std::vector& indices, bool ascending) const { align_indices(indices); - auto b = indices.begin(); - auto e = indices.end(); - std::sort(b, e, [this, ascending](size_t i1, size_t i2) { - auto k1 = get_key(i1); - auto k2 = get_key(i2); - return ascending ? k1 < k2 : k2 < k1; - }); + do_sort(indices, ascending, get_keys(*this)); } void Dictionary::distinct_keys(std::vector& indices, util::Optional) const @@ -1065,34 +1081,38 @@ Dictionary::Iterator::Iterator(const Dictionary* dict, size_t pos) { } -auto Dictionary::Iterator::operator*() const -> value_type +Mixed Dictionary::Iterator::key() const { - update(); - Mixed key; switch (m_key_type) { case type_String: { ArrayString keys(m_tree.get_alloc()); ref_type ref = to_ref(Array::get(m_leaf.get_mem().get_addr(), 1)); keys.init_from_ref(ref); - key = Mixed(keys.get(m_state.m_current_index)); - break; + return Mixed(keys.get(m_state.m_current_index)); } case type_Int: { ArrayInteger keys(m_tree.get_alloc()); ref_type ref = to_ref(Array::get(m_leaf.get_mem().get_addr(), 1)); keys.init_from_ref(ref); - key = Mixed(keys.get(m_state.m_current_index)); - break; + return Mixed(keys.get(m_state.m_current_index)); } default: throw std::runtime_error("Not implemented"); - break; } +} + +Mixed Dictionary::Iterator::value() const +{ ArrayMixed values(m_tree.get_alloc()); ref_type ref = to_ref(Array::get(m_leaf.get_mem().get_addr(), 2)); values.init_from_ref(ref); + return values.get(m_state.m_current_index); +} - return std::make_pair(key, values.get(m_state.m_current_index)); +auto Dictionary::Iterator::operator*() const -> value_type +{ + update(); + return std::make_pair(key(), value()); } diff --git a/src/realm/dictionary.hpp b/src/realm/dictionary.hpp index 50a4e6cd9da..c2d501df0f0 100644 --- a/src/realm/dictionary.hpp +++ b/src/realm/dictionary.hpp @@ -227,6 +227,9 @@ class Dictionary::Iterator : public ClusterTree::Iterator { return ret; } + Mixed key() const; + Mixed value() const; + private: friend class Dictionary; using ClusterTree::Iterator::get_position; diff --git a/test/benchmark-common-tasks/main.cpp b/test/benchmark-common-tasks/main.cpp index 5c9046f79de..e737e8a2e6e 100644 --- a/test/benchmark-common-tasks/main.cpp +++ b/test/benchmark-common-tasks/main.cpp @@ -1268,7 +1268,7 @@ struct BenchmarkSortIntList : Benchmark { struct BenchmarkSortIntDictionary : Benchmark { // Dictionary sorting is sufficiently slow that we want to use a smaller size - static const size_t size = BASE_SIZE / 10; + static const size_t size = BASE_SIZE / 2; const char* name() const {