Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Try less memory usage #516

Merged
merged 7 commits into from
Jul 28, 2023
Merged

Try less memory usage #516

merged 7 commits into from
Jul 28, 2023

Conversation

MBkkt
Copy link
Contributor

@MBkkt MBkkt commented May 26, 2023

0.4 vs 0.5 on my machine with old memset.
And 0.1 without unnecessary memset

@MBkkt MBkkt requested review from gnusi and Dronplane May 26, 2023 22:03
@MBkkt MBkkt marked this pull request as draft May 26, 2023 22:10
@@ -82,45 +82,46 @@ class column final : public irs::column_output {

class address_table {
public:
address_table(ManagedTypedAllocator<uint64_t> alloc) : alloc_{alloc} {
offsets_ = alloc_.allocate(kBlockSize);
Copy link
Contributor Author

@MBkkt MBkkt Jul 21, 2023

Choose a reason for hiding this comment

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

Now we allocate very good aligned blocks, so I think it's better utilize allocator.

Also if allocator decide to use mmap, we will get lazy init

Copy link
Contributor

Choose a reason for hiding this comment

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

Here would be 2 allocations. One for deque itself and this one for internal contents. Better to benchmark this on test load with many columns.

@MBkkt MBkkt marked this pull request as ready for review July 21, 2023 13:18
IRS_ASSERT(offset_ > offsets_);
*--offset_ = 0;
IRS_ASSERT(offsets_ < offset_);
--offset_;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't need to set zero to unused data


void reset() noexcept {
std::memset(offsets_, 0, sizeof offsets_);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't need to set zero to unused data


private:
uint64_t offsets_[kBlockSize]{};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need zero unused data

offset_ = offsets_;
}

~address_table() { alloc_.deallocate(offsets_, kBlockSize); }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also deque with columns works better now:
blocks in deque could handle few columns and we don't store this memory forever.
only while write segment

Copy link
Contributor

@Dronplane Dronplane left a comment

Choose a reason for hiding this comment

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

LGTM

@Dronplane Dronplane merged commit d0d7aa3 into master Jul 28, 2023
1 check passed
MBkkt added a commit that referenced this pull request Aug 14, 2023
* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark
MBkkt added a commit that referenced this pull request Aug 14, 2023
* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark
MBkkt added a commit that referenced this pull request Aug 14, 2023
* Try less memory usage (#516)

* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark

* Make reserve for wildcard nfa construction (#536)

* Fix burst trie UB (#537)

* Fix UB

* Hotfix for directory resource managment

* Try less memory usage (#516)

* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark

* wip (#543)

* wip

* wip

* wip
MBkkt added a commit that referenced this pull request Aug 14, 2023
* Try less memory usage (#516)

* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark

* Make reserve for wildcard nfa construction (#536)

* Fix burst trie UB (#537)

* Fix UB

* Hotfix for directory resource managment

* Try less memory usage (#516)

* Try less memory usage

* fixes

* Update columnstore2.cpp

* Add benchmark

* Add benchmark

* Add benchmark

* wip (#543)

* wip

* wip

* wip
@MBkkt MBkkt mentioned this pull request Aug 14, 2023
@MBkkt MBkkt mentioned this pull request Aug 14, 2023
MBkkt added a commit that referenced this pull request Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants