-
Notifications
You must be signed in to change notification settings - Fork 409
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
[FLASH-572] Use jemalloc instead of tcmalloc by default #424
Conversation
Signed-off-by: JaySon-Huang <[email protected]>
/run-integration-tests |
Signed-off-by: JaySon-Huang <[email protected]>
Signed-off-by: JaySon-Huang <[email protected]>
/run-integration-tests |
2 similar comments
/run-integration-tests |
/run-integration-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -137,8 +141,10 @@ StorageDeltaMerge::StorageDeltaMerge(const String & path_, | |||
void StorageDeltaMerge::drop() | |||
{ | |||
shutdown(); | |||
#if USE_TCMALLOC | |||
// Reclaim memory. | |||
MallocExtension::instance()->ReleaseFreeMemory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why jemalloc does not need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tcmalloc, clickhouse have to tune "tcmalloc.aggressive_memory_decommit" to be false to get better performance.
This piece of code is just for tcmalloc, after drop table, remind tcmalloc to reclaim free memory, or it acts like to be leak. See ClickHouse/ClickHouse#1742 for details.
But with jemalloc, no tuning is required:
When using jemalloc, there are no such issues even ** without any tuning. **
Some queries will run faster up to 20%.
Memory usage (RSS) is about 10% lower and is more stable.
/run-integration-tests |
Signed-off-by: JaySon-Huang <[email protected]>
/run-integration-tests |
3 similar comments
/run-integration-tests |
/run-integration-tests |
/run-integration-tests |
This PR
contrib/jemalloc
,contrib/jemalloc-cmake
from clickhouse and use jemalloc under Linux by defaultSome background handlers need to be removed in TMTContext's BackgroundService and Storages
Context.cpp:L148
AlterCommands.cpp:L186