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

[pinskia at gmail dot com]

> On Oct 29, 2014, at 6:43 AM, 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?
> 
> ChangeLog:
> 2014-10-29  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>    * malloc/malloc.c (__libc_malloc): Add support for aligning large blocks.
>    * sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h: New file.
>    New defines (MALLOC_LARGE_BLOCK_ALIGN), (MALLOC_LARGE_BLOCK_SIZE).
>    * 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
> +
>   void *(*hook) (size_t, const void *)
>     = atomic_forced_read (__malloc_hook);
>   if (__builtin_expect (hook != NULL, 0))
> diff --git a/sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h
> b/sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h
> new file mode 100644
> index 0000000..3fe9f72
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/malloc-sysdep.h
> @@ -0,0 +1,23 @@
> +/* System-specific malloc support functions.  AArch64 version.
> +   Copyright (C) 2012-2014 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define MALLOC_LARGE_BLOCK_ALIGN (64)

Yes that is not the only cache line size on aarch64. Why don't you make it a global variable and read it from the system register like we do for memset?  ThunderX has a cache line size of 128. 

Thanks,
Andrew


> +#define MALLOC_LARGE_BLOCK_SIZE (16384)
> +
> +#include_next <malloc-sysdep.h>
> +
> diff --git a/sysdeps/unix/sysv/linux/arm/malloc-sysdep.h
> b/sysdeps/unix/sysv/linux/arm/malloc-sysdep.h
> new file mode 100644
> index 0000000..3fe9f72
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/arm/malloc-sysdep.h
> @@ -0,0 +1,23 @@
> +/* System-specific malloc support functions.  AArch64 version.
> +   Copyright (C) 2012-2014 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define MALLOC_LARGE_BLOCK_ALIGN (64)
> +#define MALLOC_LARGE_BLOCK_SIZE (16384)
> +
> +#include_next <malloc-sysdep.h>
> +
> -- 
> 1.7.9.5
> 
>