Stop checking head state in the merge hook.

Now that all merging go through try_acquire_edata_neighbor, the mergeablility
checks (including head state checking) are done before reaching the merge hook.
In other words, merge hook will never be called if the head state doesn't agree.
This commit is contained in:
Qi Wang 2021-03-11 23:41:51 -08:00 committed by Qi Wang
parent 49b7d7f0a4
commit add636596a
5 changed files with 42 additions and 56 deletions

View File

@ -61,8 +61,7 @@ bool ehooks_default_split_impl();
bool ehooks_default_merge(extent_hooks_t *extent_hooks, void *addr_a,
size_t size_a, void *addr_b, size_t size_b, bool committed,
unsigned arena_ind);
bool ehooks_default_merge_impl(tsdn_t *tsdn, void *addr_a, bool head_a,
void *addr_b, bool head_b);
bool ehooks_default_merge_impl(tsdn_t *tsdn, void *addr_a, void *addr_b);
void ehooks_default_zero_impl(void *addr, size_t size);
/*
@ -338,21 +337,10 @@ ehooks_split(tsdn_t *tsdn, ehooks_t *ehooks, void *addr, size_t size,
static inline bool
ehooks_merge(tsdn_t *tsdn, ehooks_t *ehooks, void *addr_a, size_t size_a,
bool head_a, void *addr_b, size_t size_b, bool head_b, bool committed) {
void *addr_b, size_t size_b, bool committed) {
extent_hooks_t *extent_hooks = ehooks_get_extent_hooks_ptr(ehooks);
/*
* The definition of extent_hooks merge function doesn't know about
* extent head state, but the implementation does. As a result, it
* needs to call iealloc again and walk the rtree. Since the cost of an
* iealloc is large relative to the cost of the default merge hook
* (which on posix-likes is just "return false"), we go even further
* when we short-circuit; we don't just check if the extent hooks
* generally are default, we check if the merge hook specifically is.
*/
if (extent_hooks == &ehooks_default_extent_hooks
|| extent_hooks->merge == &ehooks_default_merge) {
return ehooks_default_merge_impl(tsdn, addr_a, head_a, addr_b,
head_b);
if (extent_hooks == &ehooks_default_extent_hooks) {
return ehooks_default_merge_impl(tsdn, addr_a, addr_b);
} else if (extent_hooks->merge == NULL) {
return true;
} else {

View File

@ -224,6 +224,26 @@ extent_assert_can_expand(const edata_t *original, const edata_t *expand) {
assert(edata_past_get(original) == edata_base_get(expand));
}
JEMALLOC_ALWAYS_INLINE bool
edata_neighbor_head_state_mergeable(bool edata_is_head,
bool neighbor_is_head, bool forward) {
/*
* Head states checking: disallow merging if the higher addr extent is a
* head extent. This helps preserve first-fit, and more importantly
* makes sure no merge across arenas.
*/
if (forward) {
if (neighbor_is_head) {
return false;
}
} else {
if (edata_is_head) {
return false;
}
}
return true;
}
JEMALLOC_ALWAYS_INLINE edata_t *
emap_edata_lookup(tsdn_t *tsdn, emap_t *emap, const void *ptr) {
EMAP_DECLARE_RTREE_CTX;

View File

@ -188,11 +188,10 @@ ehooks_default_split(extent_hooks_t *extent_hooks, void *addr, size_t size,
}
bool
ehooks_default_merge_impl(tsdn_t *tsdn, void *addr_a, bool head_a, void *addr_b,
bool head_b) {
ehooks_default_merge_impl(tsdn_t *tsdn, void *addr_a, void *addr_b) {
assert(addr_a < addr_b);
/*
* For non-DSS cases (first 2 branches) --
* For non-DSS cases --
* a) W/o maps_coalesce, merge is not always allowed (Windows):
* 1) w/o retain, never merge (first branch below).
* 2) with retain, only merge extents from the same VirtualAlloc
@ -204,17 +203,23 @@ ehooks_default_merge_impl(tsdn_t *tsdn, void *addr_a, bool head_a, void *addr_b,
* disallowed if b is a head extent, i.e. no merging across
* different mmap regions.
*
* a2) and b2) share the implementation (the no_merge_heads branch).
* a2) and b2) are implemented in emap_try_acquire_edata_neighbor, and
* sanity checked in the second branch below.
*/
if (!maps_coalesce && !opt_retain) {
return true;
}
/*
* Don't merge across mappings when retain is on -- this preserves
* first-fit ordering.
*/
if (opt_retain && head_b) {
return true;
if (config_debug) {
edata_t *a = emap_edata_lookup(tsdn, &arena_emap_global,
addr_a);
bool head_a = edata_is_head_get(a);
edata_t *b = emap_edata_lookup(tsdn, &arena_emap_global,
addr_b);
bool head_b = edata_is_head_get(b);
emap_assert_mapped(tsdn, &arena_emap_global, a);
emap_assert_mapped(tsdn, &arena_emap_global, b);
assert(edata_neighbor_head_state_mergeable(head_a, head_b,
/* forward */ true));
}
if (have_dss && !extent_dss_mergeable(addr_a, addr_b)) {
return true;
@ -228,14 +233,7 @@ ehooks_default_merge(extent_hooks_t *extent_hooks, void *addr_a, size_t size_a,
void *addr_b, size_t size_b, bool committed, unsigned arena_ind) {
tsdn_t *tsdn = tsdn_fetch();
edata_t *a = emap_edata_lookup(tsdn, &arena_emap_global, addr_a);
bool head_a = edata_is_head_get(a);
edata_t *b = emap_edata_lookup(tsdn, &arena_emap_global, addr_b);
bool head_b = edata_is_head_get(b);
emap_assert_mapped(tsdn, &arena_emap_global, a);
emap_assert_mapped(tsdn, &arena_emap_global, b);
return ehooks_default_merge_impl(tsdn, addr_a, head_a, addr_b, head_b);
return ehooks_default_merge_impl(tsdn, addr_a, addr_b);
}
void

View File

@ -48,26 +48,6 @@ emap_update_edata_state(tsdn_t *tsdn, emap_t *emap, edata_t *edata,
emap_assert_mapped(tsdn, emap, edata);
}
static inline bool
edata_neighbor_head_state_mergeable(bool edata_is_head,
bool neighbor_is_head, bool forward) {
/*
* Head states checking: disallow merging if the higher addr extent is a
* head extent. This helps preserve first-fit, and more importantly
* makes sure no merge across arenas.
*/
if (forward) {
if (neighbor_is_head) {
return false;
}
} else {
if (edata_is_head) {
return false;
}
}
return true;
}
static inline bool
edata_can_acquire_neighbor(edata_t *edata, rtree_contents_t contents,
extent_pai_t pai, extent_state_t expected_state, bool forward,

View File

@ -1189,8 +1189,8 @@ extent_merge_impl(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, edata_t *a,
emap_assert_mapped(tsdn, pac->emap, b);
bool err = ehooks_merge(tsdn, ehooks, edata_base_get(a),
edata_size_get(a), edata_is_head_get(a), edata_base_get(b),
edata_size_get(b), edata_is_head_get(b), edata_committed_get(a));
edata_size_get(a), edata_base_get(b), edata_size_get(b),
edata_committed_get(a));
if (err) {
return true;