-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Adds comptime option to link jemalloc instead of tcmalloc #44113
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
Changes from all commits
d9431fe
d51084e
2c544c3
12e17f5
52b5051
8cb983c
f9d450c
b0eb002
e44c0c3
ebcafce
45373e2
645e908
cee0753
e3ba951
b0cff72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -227,6 +227,16 @@ config_setting( | |
| values = {"define": "tcmalloc=gperftools"}, | ||
| ) | ||
|
|
||
| bool_flag( | ||
| name = "jemalloc", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it be
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah i did that initially, and thought it's just weird. Either works for me but will leave it to @phlax for the final call
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can use a string_flag similar to how the define works - ie something like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i would rather keep the bool flag as is for this PR, then refactor the flags in a follow. wdyt?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sgtm |
||
| build_setting_default = False, | ||
| ) | ||
|
|
||
| config_setting( | ||
| name = "jemalloc_enabled", | ||
| flag_values = {":jemalloc": "True"}, | ||
| ) | ||
|
|
||
| config_setting( | ||
| name = "disable_signal_trace", | ||
| values = {"define": "signal_trace=disabled"}, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,10 @@ | |
| #include "tcmalloc/malloc_extension.h" | ||
| #elif defined(GPERFTOOLS_TCMALLOC) | ||
| #include "gperftools/malloc_extension.h" | ||
| #elif defined(JEMALLOC) | ||
| #include <jemalloc/jemalloc.h> | ||
|
|
||
| #include <cstdio> | ||
| #endif | ||
|
|
||
| namespace Envoy { | ||
|
|
@@ -19,6 +23,13 @@ void Utils::releaseFreeMemory(uint64_t max_unfreed_bytes) { | |
| #elif defined(GPERFTOOLS_TCMALLOC) | ||
| UNREFERENCED_PARAMETER(max_unfreed_bytes); | ||
| MallocExtension::instance()->ReleaseFreeMemory(); | ||
| #elif defined(JEMALLOC) | ||
| UNREFERENCED_PARAMETER(max_unfreed_bytes); | ||
| // Purge all arenas to release dirty pages back to the OS. | ||
| // `MALLCTL_ARENAS_ALL` is jemalloc's pseudo-index for addressing all arenas at once. | ||
| char purge_cmd[32]; | ||
| snprintf(purge_cmd, sizeof(purge_cmd), "arena.%u.purge", MALLCTL_ARENAS_ALL); | ||
| mallctl(purge_cmd, nullptr, nullptr, nullptr, 0); | ||
|
Comment on lines
+28
to
+32
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the only place I need to learn things more about jemalloc, but it looks correct too me |
||
| #else | ||
| UNREFERENCED_PARAMETER(max_unfreed_bytes); | ||
| #endif | ||
|
|
@@ -31,7 +42,7 @@ void Utils::releaseFreeMemory(uint64_t max_unfreed_bytes) { | |
| Ref: https://github.com/envoyproxy/envoy/pull/9471#discussion_r363825985 | ||
| */ | ||
| void Utils::tryShrinkHeap() { | ||
| #if defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC) | ||
| #if defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC) || defined(JEMALLOC) | ||
| auto total_physical_bytes = Stats::totalPhysicalBytes(); | ||
| auto allocated_size_by_app = Stats::totalCurrentlyAllocated(); | ||
| const uint64_t threshold = maxUnfreedMemoryBytes(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -987,6 +987,7 @@ iteratively | |
| javascript | ||
| jitter | ||
| jittered | ||
| jemalloc | ||
| joinable | ||
| js | ||
| kafka | ||
|
|
||
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.
this is to be able to run jemalloc enabled tests since gperftools will conflict with jemalloc. Instead, added gperftools option explicitly in the CI script