This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PING][PATCH] calloc should not duplicate malloc logic.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: libc-alpha at sourceware dot org
- Date: Wed, 26 Feb 2014 12:26:16 +0100
- Subject: [PING][PATCH] calloc should not duplicate malloc logic.
- Authentication-results: sourceware.org; auth=none
- References: <20140221150417 dot GA4198 at domone dot podge>
ping
On Fri, Feb 21, 2014 at 04:04:17PM +0100, OndÅej BÃlka wrote:
> Hi,
>
> To make future improvements of allocator simpler we could for now calloc
> just call malloc and memset. With that we could omit a changes that
> would duplicate malloc changes anyway.
>
> It would temporarily decrease its performance, which is not primary
> concern now as we just started release cycle.
>
> In long time horizon it would improve maintainability and performance,
> my plan now is add a parameter used_by_calloc to internal malloc to
> keep logic together. We could not do it without some refactoring as
> int_malloc now has arena in parameter which is thing that I want to
> deduplicate.
>
> OK to commit?
>
> * malloc/malloc.c (__libc_calloc): Simplify implementation.
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 9a45707..cac08bf 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3141,14 +3141,9 @@ __libc_pvalloc (size_t bytes)
> void *
> __libc_calloc (size_t n, size_t elem_size)
> {
> - mstate av;
> - mchunkptr oldtop, p;
> - INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;
> + INTERNAL_SIZE_T bytes;
> void *mem;
> - unsigned long clearsize;
> - unsigned long nclears;
> - INTERNAL_SIZE_T *d;
> -
> +
> /* size_t is unsigned so the behavior on overflow is defined. */
> bytes = n * elem_size;
> #define HALF_INTERNAL_SIZE_T \
> @@ -3166,113 +3161,15 @@ __libc_calloc (size_t n, size_t elem_size)
> atomic_forced_read (__malloc_hook);
> if (__builtin_expect (hook != NULL, 0))
> {
> - sz = bytes;
> - mem = (*hook)(sz, RETURN_ADDRESS (0));
> - if (mem == 0)
> - return 0;
> -
> - return memset (mem, 0, sz);
> - }
> -
> - sz = bytes;
> -
> - arena_get (av, sz);
> - if (!av)
> - return 0;
> -
> - /* Check if we hand out the top chunk, in which case there may be no
> - need to clear. */
> -#if MORECORE_CLEARS
> - oldtop = top (av);
> - oldtopsize = chunksize (top (av));
> -# if MORECORE_CLEARS < 2
> - /* Only newly allocated memory is guaranteed to be cleared. */
> - if (av == &main_arena &&
> - oldtopsize < mp_.sbrk_base + av->max_system_mem - (char *) oldtop)
> - oldtopsize = (mp_.sbrk_base + av->max_system_mem - (char *) oldtop);
> -# endif
> - if (av != &main_arena)
> - {
> - heap_info *heap = heap_for_ptr (oldtop);
> - if (oldtopsize < (char *) heap + heap->mprotect_size - (char *) oldtop)
> - oldtopsize = (char *) heap + heap->mprotect_size - (char *) oldtop;
> - }
> -#endif
> - mem = _int_malloc (av, sz);
> -
> -
> - assert (!mem || chunk_is_mmapped (mem2chunk (mem)) ||
> - av == arena_for_chunk (mem2chunk (mem)));
> -
> - if (mem == 0)
> - {
> - LIBC_PROBE (memory_calloc_retry, 1, sz);
> - av = arena_get_retry (av, sz);
> - if (__builtin_expect (av != NULL, 1))
> - {
> - mem = _int_malloc (av, sz);
> - (void) mutex_unlock (&av->mutex);
> - }
> - if (mem == 0)
> - return 0;
> + mem = (*hook)(bytes, RETURN_ADDRESS (0));
> }
> else
> - (void) mutex_unlock (&av->mutex);
> - p = mem2chunk (mem);
> + mem = __libc_malloc (bytes);
>
> - /* Two optional cases in which clearing not necessary */
> - if (chunk_is_mmapped (p))
> - {
> - if (__builtin_expect (perturb_byte, 0))
> - return memset (mem, 0, sz);
> -
> - return mem;
> - }
> -
> - csz = chunksize (p);
> -
> -#if MORECORE_CLEARS
> - if (perturb_byte == 0 && (p == oldtop && csz > oldtopsize))
> - {
> - /* clear only the bytes from non-freshly-sbrked memory */
> - csz = oldtopsize;
> - }
> -#endif
> -
> - /* Unroll clear of <= 36 bytes (72 if 8byte sizes). We know that
> - contents have an odd number of INTERNAL_SIZE_T-sized words;
> - minimally 3. */
> - d = (INTERNAL_SIZE_T *) mem;
> - clearsize = csz - SIZE_SZ;
> - nclears = clearsize / sizeof (INTERNAL_SIZE_T);
> - assert (nclears >= 3);
> -
> - if (nclears > 9)
> - return memset (d, 0, clearsize);
> -
> - else
> - {
> - *(d + 0) = 0;
> - *(d + 1) = 0;
> - *(d + 2) = 0;
> - if (nclears > 4)
> - {
> - *(d + 3) = 0;
> - *(d + 4) = 0;
> - if (nclears > 6)
> - {
> - *(d + 5) = 0;
> - *(d + 6) = 0;
> - if (nclears > 8)
> - {
> - *(d + 7) = 0;
> - *(d + 8) = 0;
> - }
> - }
> - }
> - }
> + if (mem == 0)
> + return 0;
>
> - return mem;
> + return memset (mem, 0, bytes);
> }
>
> /*
--
According to Microsoft, it's by design