Try experimental hualloc memory allocator from Yandex#31376
Closed
alexey-milovidov wants to merge 30 commits intomasterfrom
Closed
Try experimental hualloc memory allocator from Yandex#31376alexey-milovidov wants to merge 30 commits intomasterfrom
hualloc memory allocator from Yandex#31376alexey-milovidov wants to merge 30 commits intomasterfrom
Conversation
robot-clickhouse
added
the
pr-performance
robot-ch-test-poll4
added
the
submodule changed
| add_subdirectory(s2geometry-cmake) | ||
| endif() | ||
|
|
||
| # Put all targets defined here and in subdirectories under "contrib/<immediate-subdir>" folders in GUI-based IDEs. |
Member
Author
There was a problem hiding this comment.
This fails on "cannot set property on ALIAS target", CC @traceon
As we don't use X-Code, I temporarily removed it.
Contributor
There was a problem hiding this comment.
OK, I'll submit adjustments/fixes, after this change is merged.
Member
Author
|
Segfault on server termination. |
Member
Author
|
Something is wrong with destruction order: |
Member
Author
|
|
Member
Author
|
It is still wrong: |
Member
Author
|
There is inefficient path for allocating and deallocating memory chunks of size 1048591 bytes. |
Member
Author
|
Something is still wrong with allocations from zlib-ng. |
traceon
reviewed
Nov 19, 2021
contrib/CMakeLists.txt
Outdated
Comment on lines
-355
to
-161
| # Skip INTERFACE library targets, since FOLDER property is not available for them. | ||
| get_target_property (_target_type "${_target}" TYPE) | ||
| if (_target_type STREQUAL "INTERFACE_LIBRARY") | ||
| return () | ||
| endif () |
Contributor
There was a problem hiding this comment.
This can be changed just to this to fix the issue:
# Skip aliases and INTERFACE library targets, since FOLDER property is not available for them.
get_target_property (_target_aliased "${_target}" ALIASED_TARGET)
get_target_property (_target_type "${_target}" TYPE)
if (_target_aliased OR _target_type STREQUAL "INTERFACE_LIBRARY")
return ()
endif ()
Member
Author
|
Test finished. |
Member
Author
|
Will reopen after @pg83 will help. |
Member
|
@alexey-milovidov is it just to test this allocator, or it is considered as a real replacement? If yes, what are the benefits? |
Member
Author
|
Let's try again. The chances it will be merged are low. |
Member
Author
|
Yes, it gives some improvement, around 3% on average. |
Member
Author
|
Half of performance comparison runs were killed by OOM killer - most likely due to increased memory consumption. |
Member
Author
|
Bug related to zlib-ng is still not fixed. |
Member
Author
|
We need help to make it production ready. |
|
I can't understand - if hualloc still consumes 4x more memory as in #31376 (comment) in exchange for 10% speedup - do you expect that customers will approve that exchange?! |
Member
Author
|
It maybe due to some easy case that will be easy to fix. We just need to figure out - why it has 4x more memory consumption. |
Member
Author
|
Note: the issue with zlib-ng should be definitely trivial to fix. |
Member
Author
For some queries it gives around 1.5x speedup. |
Member
Author
|
The issue with zlib-ng is fixed. |
Member
Author
|
@Mergifyio update |
Contributor
✅ Branch has been successfully updated |
Member
Author
|
Looks like reclaim thread did not help. |
Member
Author
|
Let's finally close this PR, because it is difficult to merge. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Try experimental
huallocmemory allocator from Yandex.