Re: [PATCH 5/5] Add single-threaded path to _int_malloc

[DJ Delorie <dj at redhat dot com>]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:

> +      if (SINGLE_THREAD_P)

Would a suitable __glibc_unlikely make sense here?  I can't convince
myself that either of these two cases would be rare enough to move to
cold sections, but I posit the question anyway...

> +	      void *p = chunk2mem (victim);
> +	      alloc_perturb (p, bytes);
> +	      return p;
> +	    }

This part does not need to be duplicated.  It can be moved outside the
if (SINGLE_THREAD_P) clause completely.  It adds one jmp if the compiler
doesn't duplicate it behind the scenes, but that's no reason to have
duplicate code.

The overall duplication of code looks... undesirable... but given
there's a macro in the middle we're changing, I don't see a good way to
factor it out, unless the SINGLE_THREAD_P test is so cheap we can
instead rewrite the REMOVE_FB() macro to use it instead of duplicating
code.  Could/did you consider that?  If the benchmarks show no
significant difference, I'd prefer cleaner code over an insignificant
speedup.  Might even call SINGLE_THREAD_P once and store the result in a
register, to let the compiler optimize it better.

Remember, "fast at any cost" is not the goal.  "Fast while still
maintainable" is ;-)