Remove validation for HPA ratios
Config validation was introduced at3aae792b
with main intention to fix infinite purging loop, but it didn't actually fix the underlying problem, just masked it. Later47d69b4ea
was merged to address the same problem. Options `hpa_dirty_mult` and `hpa_hugification_threshold` have different application dimensions: `hpa_dirty_mult` applied to active memory on the shard, but `hpa_hugification_threshold` is a threshold for single pageslab (hugepage). It doesn't make much sense to sum them up together. While it is true that too high value of `hpa_dirty_mult` and too low value of `hpa_hugification_threshold` can lead to pathological behaviour, it is true for other options as well. Poor configurations might lead to suboptimal and sometimes completely unacceptable behaviour and that's OK, that is exactly the reason why they are called poor. There are other mechanism exist to prevent extreme behaviour, when we hugified and then immediately purged page, see `hpa_hugify_blocked_by_ndirty` function, which exist to prevent exactly this case. Lastly, `hpa_dirty_mult + hpa_hugification_threshold >= 1` constraint is too tight and prevents a lot of valid configurations.
This commit is contained in:
parent
0ce13c6fb5
commit
3820e38dc1
|
@ -230,7 +230,6 @@ TESTS_UNIT := \
|
|||
$(srcroot)test/unit/hook.c \
|
||||
$(srcroot)test/unit/hpa.c \
|
||||
$(srcroot)test/unit/hpa_background_thread.c \
|
||||
$(srcroot)test/unit/hpa_validate_conf.c \
|
||||
$(srcroot)test/unit/hpdata.c \
|
||||
$(srcroot)test/unit/huge.c \
|
||||
$(srcroot)test/unit/inspect.c \
|
||||
|
|
|
@ -1041,44 +1041,6 @@ obtain_malloc_conf(unsigned which_source, char readlink_buf[PATH_MAX + 1]) {
|
|||
return ret;
|
||||
}
|
||||
|
||||
static bool
|
||||
validate_hpa_ratios(void) {
|
||||
size_t hpa_threshold = fxp_mul_frac(HUGEPAGE, opt_hpa_opts.dirty_mult) +
|
||||
opt_hpa_opts.hugification_threshold;
|
||||
if (hpa_threshold > HUGEPAGE) {
|
||||
return false;
|
||||
}
|
||||
|
||||
char hpa_dirty_mult[FXP_BUF_SIZE];
|
||||
char hugification_threshold[FXP_BUF_SIZE];
|
||||
char normalization_message[256] = {0};
|
||||
fxp_print(opt_hpa_opts.dirty_mult, hpa_dirty_mult);
|
||||
fxp_print(fxp_div(FXP_INIT_INT((unsigned)
|
||||
(opt_hpa_opts.hugification_threshold >> LG_PAGE)),
|
||||
FXP_INIT_INT(HUGEPAGE_PAGES)), hugification_threshold);
|
||||
if (!opt_abort_conf) {
|
||||
char normalized_hugification_threshold[FXP_BUF_SIZE];
|
||||
opt_hpa_opts.hugification_threshold +=
|
||||
HUGEPAGE - hpa_threshold;
|
||||
fxp_print(fxp_div(FXP_INIT_INT((unsigned)
|
||||
(opt_hpa_opts.hugification_threshold >> LG_PAGE)),
|
||||
FXP_INIT_INT(HUGEPAGE_PAGES)),
|
||||
normalized_hugification_threshold);
|
||||
malloc_snprintf(normalization_message,
|
||||
sizeof(normalization_message), "<jemalloc>: Normalizing "
|
||||
"HPA settings to avoid pathological behavior, setting "
|
||||
"hpa_hugification_threshold_ratio: to %s.\n",
|
||||
normalized_hugification_threshold);
|
||||
}
|
||||
malloc_printf(
|
||||
"<jemalloc>: Invalid combination of options "
|
||||
"hpa_hugification_threshold_ratio: %s and hpa_dirty_mult: %s. "
|
||||
"These values should sum to > 1.0.\n%s", hugification_threshold,
|
||||
hpa_dirty_mult, normalization_message);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
static void
|
||||
validate_hpa_settings(void) {
|
||||
if (!hpa_supported() || !opt_hpa) {
|
||||
|
@ -1090,9 +1052,6 @@ validate_hpa_settings(void) {
|
|||
"<jemalloc>: huge page size (%zu) greater than expected."
|
||||
"May not be supported or behave as expected.", HUGEPAGE);
|
||||
}
|
||||
if (opt_hpa_opts.dirty_mult != (fxp_t)-1 && validate_hpa_ratios()) {
|
||||
had_conf_error = true;
|
||||
}
|
||||
#ifndef JEMALLOC_HAVE_MADVISE_COLLAPSE
|
||||
if (opt_hpa_opts.hugify_sync) {
|
||||
had_conf_error = true;
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
#!/bin/sh
|
||||
|
||||
export MALLOC_CONF="hpa_dirty_mult:0.001,hpa_hugification_threshold_ratio:1.0,hpa_min_purge_interval_ms:50,hpa_sec_nshards:0"
|
||||
export MALLOC_CONF="hpa_dirty_mult:0,hpa_min_purge_interval_ms:50,hpa_sec_nshards:0"
|
||||
|
||||
|
|
|
@ -1,56 +0,0 @@
|
|||
#include "test/jemalloc_test.h"
|
||||
|
||||
static bool abort_called = false;
|
||||
static void (*default_malloc_message)(void *, const char *);
|
||||
|
||||
static void
|
||||
mock_invalid_conf_abort(void) {
|
||||
abort_called = true;
|
||||
}
|
||||
|
||||
static void
|
||||
null_malloc_message(void *_1, const char* _2) {
|
||||
}
|
||||
|
||||
TEST_BEGIN(test_hpa_validate_conf) {
|
||||
test_skip_if(!hpa_supported());
|
||||
void *ptr = malloc(4096);
|
||||
/* Need to restore this here to see any possible assert messages */
|
||||
malloc_message = default_malloc_message;
|
||||
assert_true(abort_called,
|
||||
"Should have aborted due to invalid values for hpa_dirty_mult and "
|
||||
"hpa_hugification_threshold_ratio");
|
||||
free(ptr);
|
||||
}
|
||||
TEST_END
|
||||
|
||||
/*
|
||||
* We have to set `abort_conf:true` here and not via the `MALLOC_CONF`
|
||||
* environment variable in the associated shell script for this test. This is
|
||||
* because when testing on FreeBSD (where Jemalloc is the system allocator) in
|
||||
* CI configs where HPA is not supported, setting `abort_conf:true` there would
|
||||
* result in the system Jemalloc picking this up and aborting before we could
|
||||
* ever even launch the test.
|
||||
*/
|
||||
const char *malloc_conf = "abort_conf:true";
|
||||
|
||||
int
|
||||
main(void) {
|
||||
/*
|
||||
* OK, this is a sort of nasty hack. We don't want to add *another*
|
||||
* config option for HPA (the intent is that it becomes available on
|
||||
* more platforms over time, and we're trying to prune back config
|
||||
* options generally. But we'll get initialization errors on other
|
||||
* platforms if we set hpa:true in the MALLOC_CONF (even if we set
|
||||
* abort_conf:false as well). So we reach into the internals and set
|
||||
* them directly, but only if we know that we're actually going to do
|
||||
* something nontrivial in the tests.
|
||||
*/
|
||||
if (hpa_supported()) {
|
||||
default_malloc_message = malloc_message;
|
||||
malloc_message = null_malloc_message;
|
||||
opt_hpa = true;
|
||||
invalid_conf_abort = mock_invalid_conf_abort;
|
||||
}
|
||||
return test_no_reentrancy(test_hpa_validate_conf);
|
||||
}
|
|
@ -1,3 +0,0 @@
|
|||
#!/bin/sh
|
||||
|
||||
export MALLOC_CONF='tcache:false,hpa_dirty_mult:0.25,hpa_hugification_threshold_ratio:0.6'
|
Loading…
Reference in New Issue