Re: [PATCH] calloc should not duplicate malloc logic.

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Fri, Feb 21, 2014 at 04:04:17PM +0100, OndÅej BÃlka wrote:
> 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.

I don't see the point of making changes that cause performance
regressions on the basis of a future hypothetical allocator that would
hypothetically be reviewed and approved in time for 2.20 release.  It
fails the test of being a contained changeset that brings about a net
improvement in the code.

In other words, please make these changes in your private branch, do
your allocator implementation there and then start posting patches
that as a set would bring about a net improvement.  Right now, this
one patch does not.  See Alex's work on MT safety docs for an example
of how one ought to build upon and post large changes to the code
base.  It's a lot of work, but it's unavoidable if you want to keep
the tree sane for everyone.

> -  return mem;
> +  return memset (mem, 0, bytes);

You're calling memset even for maps that have been mmapped (and hence
already zeroed), which is a really horrible thing to do.  I can't
imagine any allocator implementation that would require a
simplifaction like this.

I see that the patch is already committed.  I think it should be
reverted.

Siddhesh

Attachment: pgpR33x_auKEH.pgp
Description: PGP signature