From patchwork Mon May 26 14:45:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 352534 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 66EBA14007D for ; Tue, 27 May 2014 00:45:28 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:content-type:content-transfer-encoding; q=dns; s= default; b=Tk39GIlBi41YDCwTVyhnDMT1eSzPPqcQ4ynV14/G6i/FiU0aQNNE/ hA/dT/4R24U3IB/tEh7TVnP8jbuhUUakGDwLLz/yoy6M4o1dZwr+H+WvfZo/9uLI CvtxrfrBuwkIILZ9uoHz2udKdPM+8eJPHj7H8wqXscaZUheC/3tUgw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:content-type:content-transfer-encoding; s=default; bh=6 IFjgQHMMefvSrVYXmymivBFYns=; b=xBNTbQGLA0ZNCSihL6v/YlWMgk2PtcsGQ JIkM5+dnvDaMHe4jkHRxCqJIVi0DdLxyZIcB8APle71ch/Wsot9nl0c0RlZRSi4M r4hgvAVmjw6eDiFJMJaRsBXSrSvEMjlX1hmyNGDBPiFigVCpzoAzdbetFSOTmGH7 m6VCiZ5Ksg= Received: (qmail 16709 invoked by alias); 26 May 2014 14:45:22 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 16697 invoked by uid 89); 26 May 2014 14:45:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <5383536D.5010601@redhat.com> Date: Mon, 26 May 2014 10:45:01 -0400 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: GNU C Library CC: =?UTF-8?B?T25kxZllaiBCw61sa2E=?= , Siddhesh Poyarekar Subject: [COMMITTED] malloc/malloc.c : Remove nested function mi_arena. I've committed the following patch to mechanically remove the nested function mi_arena. This is a very simple refactoring that's easy to review and prove. Which is what we want from any refactoring. We do this because it's harder to debug nested functions, and it allows non-GNU compilers to compile this code, and this code is not performance critical. The resulting code is ~100 bytes larger because the compiler has to handle the function arguments and pointers to size_t objects. This code, and in particular calling malloc_info, is not considered in the hot path of average applications. I've added `inline' to the function to indicate to the compiler that we'd like it inlined if beneficial. Tested on x86_64 with no regressions. Tested before and after with simple malloc_info application that prints stats to verify statistics look sane. Test passes. Red Hat in particular is looking to extend and change this interface to support JSON and provide better statistics, therefore we do what is minimally required here to remove the nested function, as opposed to an inlined version as suggested by Ondrej Bilka. Cheers, Carlos. 2014-05-26 Carlos O'Donell * malloc/malloc.c (mi_arena): New function. (malloc_info): Remove nested function mi_arena. Call non-nested function mi_arena. --- Cheers, Carlos. diff --git a/malloc/malloc.c b/malloc/malloc.c index 1120d4d..92ad9f9 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -4986,154 +4986,162 @@ __posix_memalign (void **memptr, size_t alignment, size_t size) } weak_alias (__posix_memalign, posix_memalign) - -int -malloc_info (int options, FILE *fp) +static inline void +mi_arena (mstate ar_ptr, + FILE *fp, + size_t *total_nblocks, + size_t *total_nfastblocks, + size_t *total_avail, + size_t *total_fastavail, + size_t *total_system, + size_t *total_max_system, + size_t *total_aspace, + size_t *total_aspace_mprotect) { - /* For now, at least. */ - if (options != 0) - return EINVAL; - int n = 0; - size_t total_nblocks = 0; - size_t total_nfastblocks = 0; - size_t total_avail = 0; - size_t total_fastavail = 0; - size_t total_system = 0; - size_t total_max_system = 0; - size_t total_aspace = 0; - size_t total_aspace_mprotect = 0; - void - mi_arena (mstate ar_ptr) - { - fprintf (fp, "\n\n", n++); + fprintf (fp, "\n\n", n++); - size_t nblocks = 0; - size_t nfastblocks = 0; - size_t avail = 0; - size_t fastavail = 0; - struct - { - size_t from; - size_t to; - size_t total; - size_t count; - } sizes[NFASTBINS + NBINS - 1]; + size_t nblocks = 0; + size_t nfastblocks = 0; + size_t avail = 0; + size_t fastavail = 0; + struct + { + size_t from; + size_t to; + size_t total; + size_t count; + } sizes[NFASTBINS + NBINS - 1]; #define nsizes (sizeof (sizes) / sizeof (sizes[0])) - mutex_lock (&ar_ptr->mutex); - - for (size_t i = 0; i < NFASTBINS; ++i) - { - mchunkptr p = fastbin (ar_ptr, i); - if (p != NULL) - { - size_t nthissize = 0; - size_t thissize = chunksize (p); + mutex_lock (&ar_ptr->mutex); + for (size_t i = 0; i < NFASTBINS; ++i) + { + mchunkptr p = fastbin (ar_ptr, i); + if (p != NULL) + { + size_t nthissize = 0; + size_t thissize = chunksize (p); - while (p != NULL) - { - ++nthissize; - p = p->fd; - } + while (p != NULL) + { + ++nthissize; + p = p->fd; + } - fastavail += nthissize * thissize; - nfastblocks += nthissize; - sizes[i].from = thissize - (MALLOC_ALIGNMENT - 1); - sizes[i].to = thissize; - sizes[i].count = nthissize; - } - else - sizes[i].from = sizes[i].to = sizes[i].count = 0; + fastavail += nthissize * thissize; + nfastblocks += nthissize; + sizes[i].from = thissize - (MALLOC_ALIGNMENT - 1); + sizes[i].to = thissize; + sizes[i].count = nthissize; + } + else + sizes[i].from = sizes[i].to = sizes[i].count = 0; - sizes[i].total = sizes[i].count * sizes[i].to; - } + sizes[i].total = sizes[i].count * sizes[i].to; + } - mbinptr bin; - struct malloc_chunk *r; + mbinptr bin; + struct malloc_chunk *r; - for (size_t i = 1; i < NBINS; ++i) - { - bin = bin_at (ar_ptr, i); - r = bin->fd; - sizes[NFASTBINS - 1 + i].from = ~((size_t) 0); - sizes[NFASTBINS - 1 + i].to = sizes[NFASTBINS - 1 + i].total - = sizes[NFASTBINS - 1 + i].count = 0; - - if (r != NULL) - while (r != bin) - { - ++sizes[NFASTBINS - 1 + i].count; - sizes[NFASTBINS - 1 + i].total += r->size; - sizes[NFASTBINS - 1 + i].from - = MIN (sizes[NFASTBINS - 1 + i].from, r->size); - sizes[NFASTBINS - 1 + i].to = MAX (sizes[NFASTBINS - 1 + i].to, - r->size); - - r = r->fd; - } + for (size_t i = 1; i < NBINS; ++i) + { + bin = bin_at (ar_ptr, i); + r = bin->fd; + sizes[NFASTBINS - 1 + i].from = ~((size_t) 0); + sizes[NFASTBINS - 1 + i].to = sizes[NFASTBINS - 1 + i].total + = sizes[NFASTBINS - 1 + i].count = 0; + + if (r != NULL) + while (r != bin) + { + ++sizes[NFASTBINS - 1 + i].count; + sizes[NFASTBINS - 1 + i].total += r->size; + sizes[NFASTBINS - 1 + i].from + = MIN (sizes[NFASTBINS - 1 + i].from, r->size); + sizes[NFASTBINS - 1 + i].to = MAX (sizes[NFASTBINS - 1 + i].to, + r->size); + + r = r->fd; + } - if (sizes[NFASTBINS - 1 + i].count == 0) - sizes[NFASTBINS - 1 + i].from = 0; - nblocks += sizes[NFASTBINS - 1 + i].count; - avail += sizes[NFASTBINS - 1 + i].total; - } + if (sizes[NFASTBINS - 1 + i].count == 0) + sizes[NFASTBINS - 1 + i].from = 0; + nblocks += sizes[NFASTBINS - 1 + i].count; + avail += sizes[NFASTBINS - 1 + i].total; + } - mutex_unlock (&ar_ptr->mutex); + mutex_unlock (&ar_ptr->mutex); - total_nfastblocks += nfastblocks; - total_fastavail += fastavail; + *total_nfastblocks += nfastblocks; + *total_fastavail += fastavail; - total_nblocks += nblocks; - total_avail += avail; + *total_nblocks += nblocks; + *total_avail += avail; - for (size_t i = 0; i < nsizes; ++i) - if (sizes[i].count != 0 && i != NFASTBINS) - fprintf (fp, " \ + for (size_t i = 0; i < nsizes; ++i) + if (sizes[i].count != 0 && i != NFASTBINS) + fprintf (fp, " \ \n", - sizes[i].from, sizes[i].to, sizes[i].total, sizes[i].count); + sizes[i].from, sizes[i].to, sizes[i].total, sizes[i].count); - if (sizes[NFASTBINS].count != 0) - fprintf (fp, "\ + if (sizes[NFASTBINS].count != 0) + fprintf (fp, "\ \n", - sizes[NFASTBINS].from, sizes[NFASTBINS].to, - sizes[NFASTBINS].total, sizes[NFASTBINS].count); + sizes[NFASTBINS].from, sizes[NFASTBINS].to, + sizes[NFASTBINS].total, sizes[NFASTBINS].count); - total_system += ar_ptr->system_mem; - total_max_system += ar_ptr->max_system_mem; + *total_system += ar_ptr->system_mem; + *total_max_system += ar_ptr->max_system_mem; - fprintf (fp, - "\n\n" - "\n" - "\n" - "\n", - nfastblocks, fastavail, nblocks, avail, - ar_ptr->system_mem, ar_ptr->max_system_mem); + fprintf (fp, + "\n\n" + "\n" + "\n" + "\n", + nfastblocks, fastavail, nblocks, avail, + ar_ptr->system_mem, ar_ptr->max_system_mem); - if (ar_ptr != &main_arena) - { - heap_info *heap = heap_for_ptr (top (ar_ptr)); - fprintf (fp, - "\n" - "\n", - heap->size, heap->mprotect_size); - total_aspace += heap->size; - total_aspace_mprotect += heap->mprotect_size; - } - else - { - fprintf (fp, - "\n" - "\n", - ar_ptr->system_mem, ar_ptr->system_mem); - total_aspace += ar_ptr->system_mem; - total_aspace_mprotect += ar_ptr->system_mem; - } + if (ar_ptr != &main_arena) + { + heap_info *heap = heap_for_ptr (top (ar_ptr)); + fprintf (fp, + "\n" + "\n", + heap->size, heap->mprotect_size); + *total_aspace += heap->size; + *total_aspace_mprotect += heap->mprotect_size; + } + else + { + fprintf (fp, + "\n" + "\n", + ar_ptr->system_mem, ar_ptr->system_mem); + *total_aspace += ar_ptr->system_mem; + *total_aspace_mprotect += ar_ptr->system_mem; + } - fputs ("\n", fp); - } + fputs ("\n", fp); +} + +int +malloc_info (int options, FILE *fp) +{ + /* For now, at least. */ + if (options != 0) + return EINVAL; + + size_t total_nblocks = 0; + size_t total_nfastblocks = 0; + size_t total_avail = 0; + size_t total_fastavail = 0; + size_t total_system = 0; + size_t total_max_system = 0; + size_t total_aspace = 0; + size_t total_aspace_mprotect = 0; if (__malloc_initialized < 0) ptmalloc_init (); @@ -5144,7 +5152,9 @@ malloc_info (int options, FILE *fp) mstate ar_ptr = &main_arena; do { - mi_arena (ar_ptr); + mi_arena (ar_ptr, fp, &total_nblocks, &total_nfastblocks, &total_avail, + &total_fastavail, &total_system, &total_max_system, + &total_aspace, &total_aspace_mprotect); ar_ptr = ar_ptr->next; } while (ar_ptr != &main_arena);