Re: [PATCH] maint: cleanup size determination in sort [was: overly aggressive memory usage by sort.c]

[Jim Meyering]

Paul Eggert wrote:
> On 06/20/2012 05:45 AM, Jim Meyering wrote:
>> Bernhard's patch was quite deliberately (as the commit log says)
>> intended to induce no semantic change.
>
> Following up on this, here's an updated version of a patch that
> should fix the problem that Jeff Janes reported in
> <http://lists.gnu.org/archive/html/coreutils/2012-06/msg00018.html>.,
> while not reintroducing bug 10877.  Jeff, can you please try this out in
> your environment?  It'd be nicer if it could be verified to work
> there.  Thanks.

Hi Jeff,
Does Paul's patch solve your problem?

>>From ad47c76273b84d64d1c612b2cc49e9679b187004 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Mon, 11 Jun 2012 02:47:05 -0700
> Subject: [PATCH] sort: by default, do not exceed 3/4 of physical memory
>
> * src/sort.c (default_sort_size): Do not exceed 3/4 of total memory.
> See Jeff Janes's bug report in
> <http://lists.gnu.org/archive/html/coreutils/2012-06/msg00018.html>.
> ---
>  src/sort.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/sort.c b/src/sort.c
> index 5a48ce6..4036059 100644
> --- a/src/sort.c
> +++ b/src/sort.c
> @@ -1404,9 +1404,10 @@ specify_nthreads (int oi, char c, char const *s)
>  static size_t
>  default_sort_size (void)
>  {
> -  /* Let SIZE be MEM, but no more than the maximum object size or
> -     system resource limits.  Don't bother to check for values like
> -     RLIM_INFINITY since in practice they are not much less than SIZE_MAX.  
> */
> +  /* Let SIZE be MEM, but no more than the maximum object size,
> +     total memory, or system resource limits.  Don't bother to check
> +     for values like RLIM_INFINITY since in practice they are not much
> +     less than SIZE_MAX.  */
>    size_t size = SIZE_MAX;
>    struct rlimit rlimit;
>    if (getrlimit (RLIMIT_DATA, &rlimit) == 0 && rlimit.rlim_cur < size)
> @@ -1433,6 +1434,10 @@ default_sort_size (void)
>    double total = physmem_total ();
>    double mem = MAX (avail, total / 8);
>
> +  /* Leave a 1/4 margin for physical memory.  */
> +  if (total * 0.75 < size)
> +    size = total * 0.75;
> +
>    /* Return the minimum of MEM and SIZE, but no less than
>       MIN_SORT_SIZE.  Avoid the MIN macro here, as it is not quite
>       right when only one argument is floating point.  */