[PATCH] malloc: Remove bin scanning from memalign (bug 30723)
DJ Delorie
dj@redhat.com
Mon Aug 14 20:49:25 GMT 2023
Florian Weimer <fweimer@redhat.com> writes:
> On the test workload (mpv --cache=yes with VP9 video decoding), the
> bin scanning has a very poor success rate (less than 2%). The tcache
> scanning has about 50% success rate, so keep that.
>
> Update comments in malloc/tst-memalign-2 to indicate the purpose
> of the tests. Even with the scanning removed, the additional
> merging opportunities since commit 542b1105852568c3ebc712225ae78b
> ("malloc: Enable merging of remainders in memalign (bug 30723)")
> are sufficient to pass the existing large bins test.
>
> Remove leftover variables from _int_free from refactoring in the
> same commit.
LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
> - mchunkptr nextchunk; /* next contiguous chunk */
> - INTERNAL_SIZE_T nextsize; /* its size */
> - int nextinuse; /* true if nextchunk is used */
> - INTERNAL_SIZE_T prevsize; /* size of previous contiguous chunk */
> - mchunkptr bck; /* misc temp for linking */
> - mchunkptr fwd; /* misc temp for linking */
No longer needed in this scope, ok.
> @@ -5032,42 +5026,6 @@ _int_realloc (mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
> ------------------------------ memalign ------------------------------
> */
>
> -/* Returns 0 if the chunk is not and does not contain the requested
> - aligned sub-chunk, else returns the amount of "waste" from
> - trimming. NB is the *chunk* byte size, not the user byte
> - size. */
> -static size_t
> -chunk_ok_for_memalign (mchunkptr p, size_t alignment, size_t nb)
> -{
> . . .
> -}
Only used for bin scanning, so no longer needed. Ok.
> - mchunkptr victim;
No longer needed, ok.
> - /* This will be set if we found a candidate chunk. */
> - victim = NULL;
> + /* Call malloc with worst case padding to hit alignment. */
> + m = (char *) (_int_malloc (av, nb + alignment + MINSIZE));
Consistent with what the remaining code expects, ok.
> + if (m == 0)
> + return 0; /* propagate failure */
Ok.
> - /* Fast bins are singly-linked, hard to remove a chunk from the middle
> - and unlikely to meet our alignment requirements. We have not done
> - any experimentation with searching for aligned fastbins. */
> - if (av != NULL)
> - {
> . . .
> - }
Ok.
> - /* Strategy: find a spot within that chunk that meets the alignment
> - request, and then possibly free the leading and trailing space.
> - This strategy is incredibly costly and can lead to external
> - fragmentation if header and footer chunks are unused. */
> -
> - if (victim != NULL)
> - {
> - p = victim;
> - m = chunk2mem (p);
> - set_inuse (p);
> - if (av != &main_arena)
> - set_non_main_arena (p);
> - }
> - else
> - {
> - /* Call malloc with worst case padding to hit alignment. */
> -
> - m = (char *) (_int_malloc (av, nb + alignment + MINSIZE));
> -
> - if (m == 0)
> - return 0; /* propagate failure */
> -
> - p = mem2chunk (m);
> - }
No longer needed, ok.
> + p = mem2chunk (m);
Ok.
> diff --git a/malloc/tst-memalign-2.c b/malloc/tst-memalign-2.c
> index f229283dbf..ecd6fa249e 100644
> --- a/malloc/tst-memalign-2.c
> +++ b/malloc/tst-memalign-2.c
> @@ -86,7 +86,8 @@ do_test (void)
> TEST_VERIFY (tcache_allocs[i].ptr1 == tcache_allocs[i].ptr2);
> }
>
> - /* Test for non-head tcache hits. */
> + /* Test for non-head tcache hits. This exercises the memalign
> + scanning code to find matching allocations. */
> for (i = 0; i < array_length (ptr); ++ i)
> {
> if (i == 4)
> @@ -113,7 +114,9 @@ do_test (void)
> free (p);
> TEST_VERIFY (count > 0);
>
> - /* Large bins test. */
> + /* Large bins test. This verifies that the over-allocated parts
> + that memalign releases for future allocations can be reused by
> + memalign itself at least in some cases. */
>
> for (i = 0; i < LN; ++ i)
> {
Ok.
More information about the Libc-alpha
mailing list