RCORE-1624 RCORE-2055 Array Classification for enabling compression #7564
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2169916 to
4936c18
Compare
9b98002 to
d3e92aa
Compare
jedelbo
approved these changes
Apr 23, 2024
There was a problem hiding this comment.
@finnschiermer promised that this typed_print would be deleted again. If not, then all these functions could be put together in one file. Then they would not be included in the binary, if not called.
src/realm/cluster.hpp
Outdated
| virtual ref_type typed_write(ref_type ref, _impl::ArrayWriterBase& out, const Table& table, bool deep, | ||
| bool only_modified, bool compress) const = 0; | ||
|
|
||
| virtual void typed_print(std::string prefix, const Table& table) const |
There was a problem hiding this comment.
Why have table as parameter? It is available through get_owning_table()
nicola-cab
commented
Apr 23, 2024
src/realm/group_writer.cpp
Outdated
| auto ref = top.get_as_ref(Group::s_evacuation_point_ndx); | ||
| REALM_ASSERT(ref); | ||
| Array::destroy(ref, m_alloc); | ||
| Array destroy_array(m_alloc); |
There was a problem hiding this comment.
This looks odd, we're only constructing the array for destroying it…
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What, How & Why?
Compression is only supported for Integer arrays (and Strings in the future).
In order to decide whether to turn on compression or not, we need to:
This PR addresses 1. Next PRs will be focused on 2.
Most importantly, the commit machinery has changed, because instead of calling
Array::writerecursively, we, now inspect the properties of the array before to mark it "ready for compression."