This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PING][PATCH] calloc should not duplicate malloc logic.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]