From patchwork Fri Aug 11 09:40:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1820191 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=aT5U7CUu; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RMf0z2r3zz1yf2 for ; Fri, 11 Aug 2023 19:40:35 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2BC393858C50 for ; Fri, 11 Aug 2023 09:40:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2BC393858C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1691746833; bh=oOXMMC5O5sbMAF84rnz1ksOaIExPeTQAptRRZSdKETw=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=aT5U7CUujdxH65qIC1FDf65ZdiS6cmHvEOTMbxNltaHOCcScC2WmtE4OlqR7sk8aG kWLpkbHQBsDPsK95Eqf0G7KM/WIN5cvBRp/1ONBnkwAhC2SiUzzsr5/y/iZTnhZWVg husAC5lDiLyjhXdcahBzqbi3WUg3lG6AqtfojEzc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DE68F3858D20 for ; Fri, 11 Aug 2023 09:40:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DE68F3858D20 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-482-CBSK3czrN_-FBCk-1PfaqA-1; Fri, 11 Aug 2023 05:40:15 -0400 X-MC-Unique: CBSK3czrN_-FBCk-1PfaqA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BD5BC18A64E0 for ; Fri, 11 Aug 2023 09:40:14 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.14]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CB87F492B0F; Fri, 11 Aug 2023 09:40:13 +0000 (UTC) To: libc-alpha@sourceware.org Cc: DJ Delorie Subject: [PATCH v2] malloc: Remove bin scanning from memalign (bug 30723) Date: Fri, 11 Aug 2023 11:40:12 +0200 Message-ID: <87r0o9x6gz.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" 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. Remove the part of malloc/tst-memalign-2 that is supposedly exercise bin scanning (supposedly because on x86-64, removing the bin scanning does not cause this part to fail). --- v2: Remove the bin scanning part of malloc/tst-memalign-2. malloc/malloc.c | 127 ++---------------------------------------------- malloc/tst-memalign-2.c | 53 -------------------- 2 files changed, 5 insertions(+), 175 deletions(-) base-commit: 542b1105852568c3ebc712225ae78b8c8ba31a78 diff --git a/malloc/malloc.c b/malloc/malloc.c index 948f9759af..9c2cab7a59 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -5082,7 +5082,6 @@ _int_memalign (mstate av, size_t alignment, size_t bytes) mchunkptr remainder; /* spare room at end to split off */ unsigned long remainder_size; /* its size */ INTERNAL_SIZE_T size; - mchunkptr victim; nb = checked_request2size (bytes); if (nb == 0) @@ -5101,129 +5100,13 @@ _int_memalign (mstate av, size_t alignment, size_t bytes) we don't find anything in those bins, the common malloc code will scan starting at 2x. */ - /* 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)); - /* 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 (m == 0) + return 0; /* propagate failure */ - if (av != NULL) - { - int first_bin_index; - int first_largebin_index; - int last_bin_index; - - if (in_smallbin_range (nb)) - first_bin_index = smallbin_index (nb); - else - first_bin_index = largebin_index (nb); - - if (in_smallbin_range (nb * 2)) - last_bin_index = smallbin_index (nb * 2); - else - last_bin_index = largebin_index (nb * 2); - - first_largebin_index = largebin_index (MIN_LARGE_SIZE); - - int victim_index; /* its bin index */ - - for (victim_index = first_bin_index; - victim_index < last_bin_index; - victim_index ++) - { - victim = NULL; - - if (victim_index < first_largebin_index) - { - /* Check small bins. Small bin chunks are doubly-linked despite - being the same size. */ - - mchunkptr fwd; /* misc temp for linking */ - mchunkptr bck; /* misc temp for linking */ - - bck = bin_at (av, victim_index); - fwd = bck->fd; - while (fwd != bck) - { - if (chunk_ok_for_memalign (fwd, alignment, nb) > 0) - { - victim = fwd; - - /* Unlink it */ - victim->fd->bk = victim->bk; - victim->bk->fd = victim->fd; - break; - } - - fwd = fwd->fd; - } - } - else - { - /* Check large bins. */ - mchunkptr fwd; /* misc temp for linking */ - mchunkptr bck; /* misc temp for linking */ - mchunkptr best = NULL; - size_t best_size = 0; - - bck = bin_at (av, victim_index); - fwd = bck->fd; - - while (fwd != bck) - { - int extra; - - if (chunksize (fwd) < nb) - break; - extra = chunk_ok_for_memalign (fwd, alignment, nb); - if (extra > 0 - && (extra <= best_size || best == NULL)) - { - best = fwd; - best_size = extra; - } - - fwd = fwd->fd; - } - victim = best; - - if (victim != NULL) - { - unlink_chunk (av, victim); - break; - } - } - - if (victim != NULL) - break; - } - } - - /* 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); - } + p = mem2chunk (m); if ((((unsigned long) (m)) % alignment) != 0) /* misaligned */ { diff --git a/malloc/tst-memalign-2.c b/malloc/tst-memalign-2.c index f229283dbf..a99fb2c6d3 100644 --- a/malloc/tst-memalign-2.c +++ b/malloc/tst-memalign-2.c @@ -40,18 +40,6 @@ static TestCase tcache_allocs[] = { }; #define TN array_length (tcache_allocs) -static TestCase large_allocs[] = { - { 23450, 64, NULL, NULL }, - { 23450, 64, NULL, NULL }, - { 23550, 64, NULL, NULL }, - { 23550, 64, NULL, NULL }, - { 23650, 64, NULL, NULL }, - { 23650, 64, NULL, NULL }, - { 33650, 64, NULL, NULL }, - { 33650, 64, NULL, NULL } -}; -#define LN array_length (large_allocs) - void *p; /* Sanity checks, ancillary to the actual test. */ @@ -113,47 +101,6 @@ do_test (void) free (p); TEST_VERIFY (count > 0); - /* Large bins test. */ - - for (i = 0; i < LN; ++ i) - { - large_allocs[i].ptr1 = memalign (large_allocs[i].alignment, large_allocs[i].size); - CHECK (large_allocs[i].ptr1, large_allocs[i].alignment); - /* Keep chunks from combining by fragmenting the heap. */ - p = malloc (512); - CHECK (p, 4); - } - - for (i = 0; i < LN; ++ i) - free (large_allocs[i].ptr1); - - /* Force the unsorted bins to be scanned and moved to small/large - bins. */ - p = malloc (60000); - - for (i = 0; i < LN; ++ i) - { - large_allocs[i].ptr2 = memalign (large_allocs[i].alignment, large_allocs[i].size); - CHECK (large_allocs[i].ptr2, large_allocs[i].alignment); - } - - count = 0; - for (i = 0; i < LN; ++ i) - { - int ok = 0; - for (j = 0; j < LN; ++ j) - if (large_allocs[i].ptr1 == large_allocs[j].ptr2) - ok = 1; - if (ok == 1) - count ++; - } - - /* The allocation algorithm is complicated outside of the memalign - logic, so just make sure it's working for most of the - allocations. This avoids possible boundary conditions with - empty/full heaps. */ - TEST_VERIFY (count > LN / 2); - return 0; }