Skip to content

Commit

Permalink
remove unnecessary callback use for ArrayWithFind::find_action() (#7095)
Browse files Browse the repository at this point in the history 
* remove unnecessary callback use for ArrayWithFind::find_action()

* remove find_action_pattern which hasn't been used in years

* add changeloge note

* lint and address comments
  • Loading branch information
@ironage
ironage authored Oct 31, 2023
1 parent 6b189dd commit 1946578
Show file tree
Hide file tree
Showing 15 changed files with 357 additions and 295 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

### Internals
* REALM_[ATMU]SAN cmake flags no longer override compilation options and can be combined with Debug|RelWithDebInfo|etc. build types. Rel[ATMU]SAN build type shortcuts are now all slightly optimized debug-based builds with sanitizers. REALM_ASAN now works with msvc (2019/2022) builds. ([PR #6911](https://github.com/realm/realm-core/pull/6911))
* Remove ArrayWithFind's ability to use a templated callback parameter. The QueryStateBase consumers now use an index and the array leaf to get the actual value if needed. This allows certain queries such as count() to not do as many lookups to the actual values and results in a small performance gain. Also remove `find_action_pattern()` which was unused for a long time. This reduction in templating throughout the query system produces a small (~100k) binary size reduction. ([#7095](https://github.com/realm/realm-core/pull/7095))

----------------------------------------------

Expand Down
34 changes: 33 additions & 1 deletion src/realm/array.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,7 @@ MemRef Array::create(Type type, bool context_flag, WidthType width_type, size_t
template <class cond, size_t bitwidth>
bool Array::find_vtable(int64_t value, size_t start, size_t end, size_t baseindex, QueryStateBase* state) const
{
return ArrayWithFind(*this).find_optimized<cond, bitwidth>(value, start, end, baseindex, state, nullptr);
return ArrayWithFind(*this).find_optimized<cond, bitwidth>(value, start, end, baseindex, state);
}


Expand Down Expand Up @@ -1285,13 +1285,26 @@ bool QueryStateCount::match(size_t, Mixed) noexcept
return (m_limit > m_match_count);
}

bool QueryStateCount::match(size_t) noexcept
{
++m_match_count;
return (m_limit > m_match_count);
}

bool QueryStateFindFirst::match(size_t index, Mixed) noexcept
{
m_match_count++;
m_state = index;
return false;
}

bool QueryStateFindFirst::match(size_t index) noexcept
{
++m_match_count;
m_state = index;
return false;
}

template <>
bool QueryStateFindAll<std::vector<ObjKey>>::match(size_t index, Mixed) noexcept
{
Expand All @@ -1303,6 +1316,16 @@ bool QueryStateFindAll<std::vector<ObjKey>>::match(size_t index, Mixed) noexcept
return (m_limit > m_match_count);
}

template <>
bool QueryStateFindAll<std::vector<ObjKey>>::match(size_t index) noexcept
{
++m_match_count;
int64_t key_value = (m_key_values ? m_key_values->get(index) : index) + m_key_offset;
m_keys.push_back(ObjKey(key_value));

return (m_limit > m_match_count);
}

template <>
bool QueryStateFindAll<IntegerColumn>::match(size_t index, Mixed) noexcept
{
Expand All @@ -1311,3 +1334,12 @@ bool QueryStateFindAll<IntegerColumn>::match(size_t index, Mixed) noexcept

return (m_limit > m_match_count);
}

template <>
bool QueryStateFindAll<IntegerColumn>::match(size_t index) noexcept
{
++m_match_count;
m_keys.add(index);

return (m_limit > m_match_count);
}
2 changes: 2 additions & 0 deletions src/realm/array.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class QueryStateFindAll : public QueryStateBase {
{
}
bool match(size_t index, Mixed) noexcept final;
bool match(size_t index) noexcept final;

private:
T& m_keys;
Expand All @@ -83,6 +84,7 @@ class QueryStateFindFirst : public QueryStateBase {
{
}
bool match(size_t index, Mixed) noexcept final;
bool match(size_t index) noexcept final;
};

class Array : public Node, public ArrayParent {
Expand Down
2 changes: 1 addition & 1 deletion src/realm/array_integer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ void ArrayIntNull::find_all(IntegerColumn* result, value_type value, size_t col_

bool ArrayIntNull::find(int cond, value_type value, size_t start, size_t end, QueryStateBase* state) const
{
return find_impl(cond, value, start, end, state, nullptr);
return find_impl(cond, value, start, end, state);
}

size_t ArrayIntNull::find_first(value_type value, size_t begin, size_t end) const
Expand Down
16 changes: 7 additions & 9 deletions src/realm/array_integer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ class ArrayInteger : public Array, public ArrayPayload {
{
return false;
}
template <class cond, class Callback>
bool find(value_type value, size_t start, size_t end, QueryStateBase* state, Callback callback) const;
template <class cond>
bool find(value_type value, size_t start, size_t end, QueryStateBase* state) const;
};

class ArrayIntNull : public Array, public ArrayPayload {
Expand Down Expand Up @@ -125,8 +125,8 @@ class ArrayIntNull : public Array, public ArrayPayload {

bool find(int cond, value_type value, size_t start, size_t end, QueryStateBase* state) const;

template <class cond, class Callback>
bool find(value_type value, size_t start, size_t end, QueryStateBase* state, Callback callback) const;
template <class cond>
bool find(value_type value, size_t start, size_t end, QueryStateBase* state) const;

// Wrappers for backwards compatibility and for simple use without
// setting up state initialization etc
Expand All @@ -147,11 +147,9 @@ class ArrayIntNull : public Array, public ArrayPayload {
void replace_nulls_with(int64_t new_null);
bool can_use_as_null(int64_t value) const;

template <class Callback>
bool find_impl(int cond, value_type value, size_t start, size_t end, QueryStateBase* state,
Callback callback) const;
template <class cond, class Callback>
bool find_impl(value_type value, size_t start, size_t end, QueryStateBase* state, Callback callback) const;
bool find_impl(int cond, value_type value, size_t start, size_t end, QueryStateBase* state) const;
template <class cond>
bool find_impl(value_type value, size_t start, size_t end, QueryStateBase* state) const;
};


Expand Down
41 changes: 18 additions & 23 deletions src/realm/array_integer_tpl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,37 +24,34 @@

namespace realm {

template <class cond, class Callback>
bool ArrayInteger::find(value_type value, size_t start, size_t end, QueryStateBase* state, Callback callback) const
template <class cond>
bool ArrayInteger::find(value_type value, size_t start, size_t end, QueryStateBase* state) const
{
return ArrayWithFind(*this).find<cond, Callback>(value, start, end, 0, state, callback);
return ArrayWithFind(*this).find<cond>(value, start, end, 0, state);
}

template <class Callback>
inline bool ArrayIntNull::find_impl(int cond, value_type value, size_t start, size_t end, QueryStateBase* state,
Callback callback) const
inline bool ArrayIntNull::find_impl(int cond, value_type value, size_t start, size_t end, QueryStateBase* state) const
{
switch (cond) {
case cond_Equal:
return find_impl<Equal>(value, start, end, state, callback);
return find_impl<Equal>(value, start, end, state);
case cond_NotEqual:
return find_impl<NotEqual>(value, start, end, state, callback);
return find_impl<NotEqual>(value, start, end, state);
case cond_Greater:
return find_impl<Greater>(value, start, end, state, callback);
return find_impl<Greater>(value, start, end, state);
case cond_Less:
return find_impl<Less>(value, start, end, state, callback);
return find_impl<Less>(value, start, end, state);
case cond_None:
return find_impl<None>(value, start, end, state, callback);
return find_impl<None>(value, start, end, state);
case cond_LeftNotNull:
return find_impl<NotNull>(value, start, end, state, callback);
return find_impl<NotNull>(value, start, end, state);
}
REALM_ASSERT_DEBUG(false);
return false;
}

template <class cond, class Callback>
bool ArrayIntNull::find_impl(value_type opt_value, size_t start, size_t end, QueryStateBase* state,
Callback callback) const
template <class cond>
bool ArrayIntNull::find_impl(value_type opt_value, size_t start, size_t end, QueryStateBase* state) const
{
int64_t null_value = Array::get(0);
bool find_null = !bool(opt_value);
Expand All @@ -79,7 +76,7 @@ bool ArrayIntNull::find_impl(value_type opt_value, size_t start, size_t end, Que
}

// Fall back to plain Array find.
return ArrayWithFind(*this).find<cond>(value, start2, end2, baseindex2, state, callback);
return ArrayWithFind(*this).find<cond>(value, start2, end2, baseindex2, state);
}
else {
cond c;
Expand All @@ -95,8 +92,7 @@ bool ArrayIntNull::find_impl(value_type opt_value, size_t start, size_t end, Que
int64_t v = Array::get(i);
bool value_is_null = (v == null_value);
if (c(v, value, value_is_null, find_null)) {
util::Optional<int64_t> v2 = value_is_null ? util::none : util::make_optional(v);
if (!ArrayWithFind(*this).find_action(i + baseindex2, v2, state, callback)) {
if (!state->match(i + baseindex2)) {
return false; // tell caller to stop aggregating/search
}
}
Expand All @@ -109,19 +105,18 @@ template <class cond>
size_t ArrayIntNull::find_first(value_type value, size_t start, size_t end) const
{
QueryStateFindFirst state;
find_impl<cond>(value, start, end, &state, nullptr);
find_impl<cond>(value, start, end, &state);

if (state.match_count() > 0)
return to_size_t(state.m_state);
else
return not_found;
}

template <class cond, class Callback>
inline bool ArrayIntNull::find(value_type value, size_t start, size_t end, QueryStateBase* state,
Callback callback) const
template <class cond>
inline bool ArrayIntNull::find(value_type value, size_t start, size_t end, QueryStateBase* state) const
{
return find_impl<cond, Callback>(value, start, end, state, callback);
return find_impl<cond>(value, start, end, state);
}

} // namespace realm
Expand Down
14 changes: 7 additions & 7 deletions src/realm/array_with_find.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void ArrayWithFind::find_all(IntegerColumn* result, int64_t value, size_t col_of
end = m_array.m_size;

QueryStateFindAll state(*result);
REALM_TEMPEX2(find_optimized, Equal, m_array.m_width, (value, begin, end, col_offset, &state, nullptr));
REALM_TEMPEX2(find_optimized, Equal, m_array.m_width, (value, begin, end, col_offset, &state));

return;
}
Expand All @@ -39,22 +39,22 @@ bool ArrayWithFind::find(int cond, int64_t value, size_t start, size_t end, size
QueryStateBase* state) const
{
if (cond == cond_Equal) {
return find<Equal>(value, start, end, baseindex, state, nullptr);
return find<Equal>(value, start, end, baseindex, state);
}
if (cond == cond_NotEqual) {
return find<NotEqual>(value, start, end, baseindex, state, nullptr);
return find<NotEqual>(value, start, end, baseindex, state);
}
if (cond == cond_Greater) {
return find<Greater>(value, start, end, baseindex, state, nullptr);
return find<Greater>(value, start, end, baseindex, state);
}
if (cond == cond_Less) {
return find<Less>(value, start, end, baseindex, state, nullptr);
return find<Less>(value, start, end, baseindex, state);
}
if (cond == cond_None) {
return find<None>(value, start, end, baseindex, state, nullptr);
return find<None>(value, start, end, baseindex, state);
}
else if (cond == cond_LeftNotNull) {
return find<NotNull>(value, start, end, baseindex, state, nullptr);
return find<NotNull>(value, start, end, baseindex, state);
}
REALM_ASSERT_DEBUG(false);
return false;
Expand Down
Loading

0 comments on commit 1946578

Please sign in to comment.