From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Should use of mallopt depend on DOUG_LEA_MALLOC? Date: Fri, 13 Dec 2019 13:55:36 -0800 Organization: UCLA Computer Science Department Message-ID: <4bd5ae89-2862-586f-7f77-cb469d12197e@cs.ucla.edu> References: <87sgme1ww7.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <83o8x0rl6d.fsf@gnu.org> <87lfs2mzo8.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <87lfs19shb.fsf@mail.linkov.net> <87a78gn5k5.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <83sgm8ox3g.fsf@gnu.org> <87zhgflxmc.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <83lfrzq1s1.fsf@gnu.org> <87d0d7w3tt.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <83zhgaloc5.fsf@gnu.org> <87zhg7jmjg.fsf@yantar92-laptop.i-did-not-set--mail-host-address--so-tickle-me> <83h82ej03y.fsf@gnu.org> <87r21ick48.fsf@gmail.com> <83immthovt.fsf@gnu.org> <87o8wkc50t.fsf@gmail.com> <83lfrodix4.fsf@gnu.org> <87immkctly.fsf_-_@gmail.com> <83tv6470ko.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------97E9CE7F7F778DB7688DA136" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="221367"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 Cc: Noam Postavsky , emacs-devel@gnu.org To: Stefan Monnier , Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Dec 13 22:56:57 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1ifsvf-000vQn-9A for ged-emacs-devel@m.gmane.org; Fri, 13 Dec 2019 22:56:55 +0100 Original-Received: from localhost ([::1]:53894 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ifsvd-0002Av-Uo for ged-emacs-devel@m.gmane.org; Fri, 13 Dec 2019 16:56:53 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55511) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ifsuY-0001UH-4f for emacs-devel@gnu.org; Fri, 13 Dec 2019 16:55:47 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ifsuV-0006JC-Uz for emacs-devel@gnu.org; Fri, 13 Dec 2019 16:55:45 -0500 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:35336) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ifsuS-00068j-QS; Fri, 13 Dec 2019 16:55:41 -0500 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 9AE4F160240; Fri, 13 Dec 2019 13:55:38 -0800 (PST) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id BOgBcVI6WCVK; Fri, 13 Dec 2019 13:55:37 -0800 (PST) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 667CE160549; Fri, 13 Dec 2019 13:55:37 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id C8vTC25tHV4o; Fri, 13 Dec 2019 13:55:37 -0800 (PST) Original-Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 4405E160240; Fri, 13 Dec 2019 13:55:37 -0800 (PST) In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:243362 Archived-At: This is a multi-part message in MIME format. --------------97E9CE7F7F778DB7688DA136 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 12/13/19 7:22 AM, Stefan Monnier wrote: > For those in `init_alloc_once_for_pdumper`, I can't help you. Those mallopt calls shouldn't be needed. > But the ones conditionalized on `mmap_lisp_allowed_p` are used not as > optimizations but to prevent use of mmap within the malloc > implementation in the two known cases where it can break our > (unportable) assumptions: Yes, we need to keep those only for the vanishingly small cases where we use unexec and a Doug Lea-like malloc. I looked through Emacs's uses of mallopt. There's a lot of cargo-cult cruft there that should go. Plus, I expect there's a real bug with newer glibc (so mallopt isn't called) but unexec is still used (because the builder asked for it - a mistake in my view) and where Emacs doesn't disable mmap properly. Proposed patch attached. The basic idea is to use "#ifdef M_MMAP_MAX" instead of "#ifdef DOUG_LEA_MALLOC" to decide whether to call mallopt with M_MMAP_MAX. Plus, stop fiddling with mallopt parameters that are problematic given that glibc malloc has mutated so much (and is likely to mutate further). --------------97E9CE7F7F778DB7688DA136 Content-Type: text/x-patch; charset=UTF-8; name="0001-Simplify-use-of-mallopt.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Simplify-use-of-mallopt.patch" >From ed60b3ae46e50f3ad214d1e2591d1afa7da651a6 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 13 Dec 2019 13:45:10 -0800 Subject: [PATCH] Simplify use of mallopt Do not mess with mallopt advice except when necessary. The advice we were giving was so old it was surely wrong now. This change also should fix some trivial performance issues. * src/alloc.c (MMAP_MAX_AREAS): Remove. All uses changed to glibc default. (pointers_fit_in_lispobj_p, mmap_lisp_allowed_p): Define if M_MMAP_MAX, not if DOUG_LEA_MALLOC. (maybe_disable_malloc_mmap, reenable_malloc_mmap): New functions, to simplify callers. Use glibc default when reenabling. (lisp_align_malloc, allocate_string_data, allocate_vectorlike): Use these functions. (lisp_align_malloc): Fix bug where mmap continued to be disabled when memory was exhausted, on platforms using unexec. (allocate_vectorlike): Do not disable and then reenable mmap when calling allocate_vector_from_block, as it allocates only small blocks. (init_alloc_once_for_pdumper): Do not alter M_MMAP_THRESHOLD or M_MMAP_MAX; the defaults are fine. * src/ralloc.c (r_alloc_init) [DOUGL_LEA_MALLOC]: Do not alter M_TOP_PAD. --- src/alloc.c | 70 +++++++++++++++++++--------------------------------- src/ralloc.c | 18 +++----------- 2 files changed, 28 insertions(+), 60 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 9fbd0d0573..4e8d7d9721 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -106,11 +106,6 @@ Copyright (C) 1985-1986, 1988, 1993-1995, 1997-2019 Free Software #ifdef DOUG_LEA_MALLOC -/* Specify maximum number of areas to mmap. It would be nice to use a - value that explicitly means "no limit". */ - -# define MMAP_MAX_AREAS 100000000 - /* A pointer to the memory allocated that copies that static data inside glibc's malloc. */ static void *malloc_state_ptr; @@ -552,7 +547,8 @@ tally_consing (ptrdiff_t nbytes) consing_until_gc -= nbytes; } -#ifdef DOUG_LEA_MALLOC +#ifdef M_MMAP_MAX + static bool pointers_fit_in_lispobj_p (void) { @@ -569,6 +565,23 @@ mmap_lisp_allowed_p (void) regions. */ return pointers_fit_in_lispobj_p () && !will_dump_with_unexec_p (); } + +/* Temporarily disable and then reenable malloc's use of mmap. */ +static void +maybe_disable_malloc_mmap (void) +{ + if (!mmap_lisp_allowed_p ()) + mallopt (M_MMAP_MAX, 0); +} +static void +reenable_malloc_mmap (void) +{ + if (!mmap_lisp_allowed_p ()) + mallopt (M_MMAP_MAX, 1<<16 /* glibc default */); +} +#else +static void maybe_disable_malloc_mmap (void) {} +static void reenable_malloc_mmap (void) {} #endif /* Head of a circularly-linked list of extant finalizers. */ @@ -1133,11 +1146,7 @@ lisp_align_malloc (size_t nbytes, enum mem_type type) int i; bool aligned; -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, 0); -#endif - + maybe_disable_malloc_mmap (); #ifdef USE_ALIGNED_ALLOC verify (ABLOCKS_BYTES % BLOCK_ALIGN == 0); abase = base = aligned_alloc (BLOCK_ALIGN, ABLOCKS_BYTES); @@ -1145,6 +1154,7 @@ lisp_align_malloc (size_t nbytes, enum mem_type type) base = malloc (ABLOCKS_BYTES); abase = pointer_align (base, BLOCK_ALIGN); #endif + reenable_malloc_mmap (); if (base == 0) { @@ -1156,11 +1166,6 @@ lisp_align_malloc (size_t nbytes, enum mem_type type) if (!aligned) ((void **) abase)[-1] = base; -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, MMAP_MAX_AREAS); -#endif - #if ! USE_LSB_TAG /* If the memory just allocated cannot be addressed thru a Lisp object's pointer, and it needs to be, that's equivalent to @@ -1807,18 +1812,9 @@ allocate_string_data (struct Lisp_String *s, if (nbytes > LARGE_STRING_BYTES) { size_t size = FLEXSIZEOF (struct sblock, data, needed); - -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, 0); -#endif - + maybe_disable_malloc_mmap (); b = lisp_malloc (size + GC_STRING_EXTRA, MEM_TYPE_NON_LISP); - -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, MMAP_MAX_AREAS); -#endif + reenable_malloc_mmap (); data = b->data; b->next = large_sblocks; @@ -3145,27 +3141,19 @@ allocate_vectorlike (ptrdiff_t len) MALLOC_BLOCK_INPUT; -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, 0); -#endif - if (nbytes <= VBLOCK_BYTES_MAX) p = allocate_vector_from_block (vroundup (nbytes)); else { + maybe_disable_malloc_mmap (); struct large_vector *lv = lisp_malloc (large_vector_offset + nbytes, MEM_TYPE_VECTORLIKE); + reenable_malloc_mmap (); lv->next = large_vectors; large_vectors = lv; p = large_vector_vec (lv); } -#ifdef DOUG_LEA_MALLOC - if (!mmap_lisp_allowed_p ()) - mallopt (M_MMAP_MAX, MMAP_MAX_AREAS); -#endif - if (find_suspicious_object_in_range (p, (char *) p + nbytes)) emacs_abort (); @@ -7302,14 +7290,6 @@ init_alloc_once_for_pdumper (void) purebeg = PUREBEG; pure_size = PURESIZE; mem_init (); - -#ifdef DOUG_LEA_MALLOC - mallopt (M_TRIM_THRESHOLD, 128 * 1024); /* Trim threshold. */ - mallopt (M_MMAP_THRESHOLD, 64 * 1024); /* Mmap threshold. */ - mallopt (M_MMAP_MAX, MMAP_MAX_AREAS); /* Max. number of mmap'ed areas. */ -#endif - - init_finalizer_list (&finalizers); init_finalizer_list (&doomed_finalizers); refill_memory_reserve (); diff --git a/src/ralloc.c b/src/ralloc.c index 66ea2ec411..857702825b 100644 --- a/src/ralloc.c +++ b/src/ralloc.c @@ -1176,23 +1176,11 @@ r_alloc_init (void) extra_bytes = PAGE_ROUNDUP (50000); #endif -#ifdef DOUG_LEA_MALLOC - block_input (); - mallopt (M_TOP_PAD, 64 * 4096); - unblock_input (); -#else -#if !defined SYSTEM_MALLOC && !defined HYBRID_MALLOC - /* Give GNU malloc's morecore some hysteresis so that we move all - the relocatable blocks much less often. The number used to be - 64, but alloc.c would override that with 32 in code that was - removed when SYNC_INPUT became the only input handling mode. - That code was conditioned on !DOUG_LEA_MALLOC, so the call to - mallopt above is left unchanged. (Actually, I think there's no - system nowadays that uses DOUG_LEA_MALLOC and also uses - REL_ALLOC.) */ +#if !defined DOUG_LEA_MALLOC && !defined HYBRID_MALLOC && !defined SYSTEM_MALLOC + /* Give gmalloc's morecore some hysteresis so that we move all + the relocatable blocks much less often. */ __malloc_extra_blocks = 32; #endif -#endif #if !defined SYSTEM_MALLOC && !defined HYBRID_MALLOC first_heap->end = (void *) PAGE_ROUNDUP (first_heap->start); -- 2.23.0 --------------97E9CE7F7F778DB7688DA136--