Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array compression with collections in Mixed #7412

Merged

Conversation

nicola-cab
Copy link
Member

What, How & Why?

Prepare Array classification + compression to core 14.0.0 which contains collections in mixed and requires some more handling in order to compress those arrays.

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)
  • C-API, if public C++ API changed
  • bindgen/spec.yml, if public C++ API changed

jedelbo and others added 30 commits November 30, 2022 13:31
Optimize storage of Decimal128 properties so that the individual values will take up 0 bits (if all nulls or all zero), 32 bits, 64 bits or 128 bits depending on what is needed.
* temporary disable failing c api decimal test
* Revert "update next major to core 13.4.1 (#6310)"

This reverts commit 59764a2.

* appease format checks
update next major to core v13.4.2
"Feature/bugfix release"
Introduce a new class - ColletionParent - which a collection will refer to as its owner. This class can be
specialized as an Obj if the nesting level is 0 or a CollectionList if the collection is nested.
Basic support for nested collections
Make it clear that when an Obj is the owner, then the index must be a ColKey
* Handle nullifying links in nested collections
* Clear backlinks related to nested collections
* dump to json support info about nested collections for schema

* reuse logic for printing nested collections

* main logic for expanding nested collections to json, requires to be polished

* more testing for nested containers

* complete algo for printing nested collections in json format

* add testing json files to project

* generate json files option set to false

* run whole test suite

* test nested collections with links

* format checks

* Move out_mixed_json... functionality to Mixed class

* Remove not needed template parameter from CollectionBaseImpl

* Delegate to_json to collections

* remove commented code

* fix audit conflicts

---------

Co-authored-by: Jørgen Edelbo <[email protected]>
* Cleanup naming and consolidate update strategy
* Allow a Mixed containing a ref to be stores in ArrayMixed
auto rot = leaf.get_as_ref_or_tagged(i);
if (rot.is_ref() && rot.get_as_ref()) {
// entries 0-2 are integral and can be compressed, entry 3 is strings and not compressed (yet)
bool do_compress = compress && i < 3;
// collections in mixed are stored at position 4.
bool do_compress = false; // (i < 3 || i == 4) ? true : false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@finnschiermer disabling Mixed compression for now, first merge this then new PR for compressing mixed and collections in mixed.

@nicola-cab nicola-cab marked this pull request as ready for review March 5, 2024 14:44
Copy link
Contributor

@finnschiermer finnschiermer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Feel free to merge.

@nicola-cab
Copy link
Member Author

LGTM. Feel free to merge.

Once CI passes I will do it :-) ..

@nicola-cab nicola-cab force-pushed the nc/array_compression_with_nested_collections branch from 613d3e1 to 4837a65 Compare March 5, 2024 15:29
Copy link

coveralls-official bot commented Mar 5, 2024

Pull Request Test Coverage Report for Build nicola.cabiddu_1458

Details

  • 10920 of 14259 (76.58%) changed or added relevant lines in 196 files are covered.
  • 640 unchanged lines in 66 files lost coverage.
  • Overall coverage decreased (-0.9%) to 90.684%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/realm/array_backlink.cpp 26 27 96.3%
src/realm/collection.cpp 92 93 98.92%
src/realm/obj.hpp 10 11 90.91%
src/realm/object-store/c_api/list.cpp 34 35 97.14%
src/realm/parser/keypath_mapping.hpp 0 1 0.0%
src/realm/query_engine.hpp 5 6 83.33%
src/realm/spec.cpp 27 28 96.43%
src/realm/table.hpp 62 63 98.41%
src/realm/util/bson/bson.cpp 11 12 91.67%
src/realm/util/logger.cpp 39 40 97.5%
Files with Coverage Reduction New Missed Lines %
src/realm/array_decimal128.hpp 1 88.89%
src/realm/array_integer_tpl.hpp 1 75.0%
src/realm/array_mixed.cpp 1 92.32%
src/realm/exceptions.cpp 1 87.78%
src/realm/impl/transact_log.hpp 1 67.97%
src/realm/index_string.hpp 1 93.68%
src/realm/list.hpp 1 84.6%
src/realm/object-store/c_api/conversion.hpp 1 81.6%
src/realm/object-store/c_api/query.cpp 1 94.36%
src/realm/replication.hpp 1 80.46%
Totals Coverage Status
Change from base Build nicola.cabiddu_1435: -0.9%
Covered Lines: 240002
Relevant Lines: 264658

💛 - Coveralls

@nicola-cab nicola-cab force-pushed the nc/array_compression_with_nested_collections branch 11 times, most recently from 331a0dc to eaa7546 Compare March 5, 2024 22:05
@nicola-cab nicola-cab force-pushed the nc/array_compression_with_nested_collections branch from eaa7546 to 4040fd7 Compare March 5, 2024 22:26
finnschiermer and others added 7 commits March 6, 2024 12:32
* tentative fix for 32 bit archs
* removed wrong cast to size_t from bf_iterator::set_value()
* fix inverted condition in 'unsigned_to_num_bits'
* fix inverted condition in 'unsigned_to_num_bits'

---------

Co-authored-by: Nicola Cabiddu <[email protected]>
This reverts commit 7ac0073.
….com:realm/realm-core into nc/array_compression_with_nested_collections
@nicola-cab nicola-cab merged commit 1eaa329 into nc/merge_all_together Mar 7, 2024
26 of 29 checks passed
@nicola-cab nicola-cab deleted the nc/array_compression_with_nested_collections branch March 7, 2024 16:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.