Re: [RFC PATCH]: Align large allocations to cacheline

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

On Wed, Oct 29, 2014 at 2:05 PM, Will Newton <will.newton@linaro.org> wrote:
> On 29 October 2014 13:43, Wilco Dijkstra <wdijkstr@arm.com> wrote:
>> This patch aligns allocations of large blocks to a cacheline on ARM and AArch64. The main goal is to
>> reduce performance variations due to random alignment choices, however it improves performance on
>> several benchmarks as well. SPECFP2000 improves by ~1.5%.
>>
>> Any comments?
>
> This bug may be relevant to changing malloc alignments:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=6527
>
> I don't know precisely what the issue is with malloc_get_state so I am
> not sure if it applies here.
>
>> ChangeLog:
>> 2014-10-29  Wilco Dijkstra  <wdijkstr@arm.com>
>>
>>         * malloc/malloc.c (__libc_malloc): Add support for aligning large blocks.
>
> I think this should be:
>
> malloc/malloc.c (__libc_malloc) [MALLOC_LARGE_BLOCK_ALIGN]: Add
> support for aligning large blocks.
>
> Although I am not a GNU ChangeLog expert...
>
>>         * sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h: New file.
>>         New defines (MALLOC_LARGE_BLOCK_ALIGN), (MALLOC_LARGE_BLOCK_SIZE).
>
> "New file." should be sufficient.
>
>>         * sysdeps/unix/sysv/linux/arm/malloc-sysdep.h: Likewise.
>>
>> ---
>>  malloc/malloc.c                                 |    8 ++++++++
>>  sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h |   23 +++++++++++++++++++++++
>>  sysdeps/unix/sysv/linux/arm/malloc-sysdep.h     |   23 +++++++++++++++++++++++
>>  3 files changed, 54 insertions(+)
>>  create mode 100644 sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h
>>  create mode 100644 sysdeps/unix/sysv/linux/arm/malloc-sysdep.h
>>
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index 6cbe9f3..0b0466e 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -2878,6 +2878,14 @@ __libc_malloc (size_t bytes)
>>    mstate ar_ptr;
>>    void *victim;
>>
>> +#ifdef MALLOC_LARGE_BLOCK_ALIGN
>> +  if (bytes > MALLOC_LARGE_BLOCK_SIZE)
>> +    {
>> +      void *address = RETURN_ADDRESS (0);
>> +      return _mid_memalign (MALLOC_LARGE_BLOCK_ALIGN, bytes, address);
>> +    }
>> +#endif
>> +
>
> This will change the behaviour of malloc hooks i.e. the malloc hook
> will not get triggered but memalign will.

Yeah that was my first reaction too - but read _mid_memalign - that
appears to call the malloc hooks too ?


Ramana