Re: [PING] [v9] malloc: Consistently apply trim_threshold to all heaps [BZ #17195]

[Mel Gorman <mgorman at suse dot de>]

On Thu, Apr 30, 2015 at 10:36:06PM -0400, Carlos O'Donell wrote:
> On 04/02/2015 03:17 AM, Mel Gorman wrote:
> > On Thu, Apr 02, 2015 at 02:47:13AM -0400, Siddhesh Poyarekar wrote:
> >> ----- Original Message -----
> >>>>>>> Ping as it's been a while since anyone said anything on it. I note
> >>>>>>> it's
> >>>>>>> in patchwork (http://patchwork.sourceware.org/patch/5496/) but do not
> >>>>>>> know if that means anyone plans to look at it. Thanks.
> >>
> >> I've tested and pushed this patch now.  In future please also run the glibc
> >> testsuite and mention in your submission what the results were.
> >>
> > 
> > Thanks very much. FWIW, the test suite was run and there was no change in the
> > number of passes or failures. I'll put that in the changelog in the future.
> 
> Siddhesh, Mel,
> 
> Sorry, I disappeared for vacation and just got back,

Hope it was a good one!

> but I did test the
> patch against some real workloads, and wanted to propose a slight tweak
> to what we have checked in. I would appreciate your opinions on the
> change.
> 

I'd be interested in hearing about which workload motivated this for future
reference. I'm not 100% happy with the ones I used (mostly artificial
microbenches or proprietary workloads). The research papers for malloc
either used poor workloads or the source was unavailable but I could have
missed something.

Also FWIW, my trim patch is still making its way into opensuse. It got held
up by a bug in another package that was using uninitialsed memory.  In the
future if there is a problem that points the finger at this patch, it's
worth testing with MALLOC_CHECK_ to see if that "fixes" it by zeroing memory.

> In _int_free, and systrim we have similar patterns to finding a chunk
> and freeing it, but in _int_free the logic is slightly different and
> my suggestion harmonizes with it:
> 
> malloc/malloc.c (_int_free):
> 
> 4042       if (av == &main_arena) {
> 4043 #ifndef MORECORE_CANNOT_TRIM
> 4044         if ((unsigned long)(chunksize(av->top)) >=
> 4045             (unsigned long)(mp_.trim_threshold))
> 4046           systrim(mp_.top_pad, av);
> 4047 #endif
> 
> The trimming in _int_free is *only* done if the top chunk is at least
> the size of the trim threshold.
> 
> This is slightly different to what we are doing right now in heap_trim
> which is to check if the extra space is at least the trim size. It is more
> precise to say "trim if the top chunk is at least the trim threshold"
> and it still fixes the issue at hand.
> 
> What do you think?
> 

I cannot think of any problems this might cause and it's better to be
consistent.

-- 
Mel Gorman
SUSE Labs