RCORE-2065 use compressed string view for quick comparison if the leaf is compressed #7880
Conversation
Pull Request Test Coverage Report for Build nicola.cabiddu_1844Details
💛 - Coveralls |
src/realm/query_engine.hpp
Outdated
| StringData t = get_string(s); | ||
| if constexpr (case_sensitive_comparison) { | ||
| // case insensitive not implemented for: >, >=, <, <= | ||
| if (cond(t, m_string_value)) | ||
| return s; | ||
| } | ||
| else { | ||
| StringData t = get_string(s); |
There was a problem hiding this comment.
for operators like BeginsWith and EndsWith. Especially for case-insensitive comparison.
There was a problem hiding this comment.
The same line of code exists 7 lines up, so it seems redundant?
|
|
||
| // special handling for !=, <, <= , >, >= if the leaf is compressed and we have got a compressed string | ||
| // id. | ||
| if constexpr (realm::is_any_v<TConditionFunction, NotEqual, Greater, Less, GreaterEqual, LessEqual>) { |
There was a problem hiding this comment.
All of these operators provide an integer overload for bool operator()(int64_t v1, int64_t v2) const which already does what you want, no need to reimplement it here. Just call it with the result of the comparison, and the other argument can be 0.
src/realm/string_compressor.cpp
Outdated
| } | ||
| // The compressed strings are identical or one is the prefix of the other | ||
| return B.size - A.size; | ||
| return A.size - B.size; |
There was a problem hiding this comment.
Please make sure the comment below matches this change.
I'm also not a fan of such implicit unsigned to signed casting because the result is actually a large unsigned number when A.size is < B.size and we rely on the type conversion to do the right thing here. I think it does work for all platforms we care about, but if for some reason the return type changed to int64_t then this code would be incorrect.
There was a problem hiding this comment.
Agreed, I can change the comment and maybe add an explicit cast. I don't think we can ever have a platform where int matches with int64_t, but never say never.
What, How & Why?
Optimize string searching which are not for equality.
Current search relies on decompressing strings and comparing them via
StringDatacomparison operator.We can instead rely on the
CompressedStringViewif the leaf is compressed, and we have got acompressed_string_idfor the currentStringNode.☑️ ToDos
bindgen/spec.yml, if public C++ API changed