This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Inline useless nested function mi_arena.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>, OndÅej BÃlka <neleai at seznam dot cz>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 20 May 2014 17:28:31 -0400
- Subject: Re: [PATCH] Inline useless nested function mi_arena.
- Authentication-results: sourceware.org; auth=none
- References: <20140226214924 dot GA10204 at domone dot podge> <20140520154152 dot GH14500 at spoyarek dot pnq dot redhat dot com>
On 05/20/2014 11:41 AM, Siddhesh Poyarekar wrote:
> On Wed, Feb 26, 2014 at 10:49:24PM +0100, OndÅej BÃlka wrote:
>> Hi,
>>
>> I noticed a minor simplification in malloc_info, that nested function
>> mi_arena looks useless as it gets called at only one site and I do not
>> have to rename its argument. The resulting patch is big due of
>> reindenting but is simple in principle.
>>
>> Is there any deep reason to keep it?
>
> I agree that it makes more sense to just inline it since there's just
> one call site and especially since breaking it out into a function
> will result in a lot of state being passed on to the function, which
> is a waste. If Carlos agrees, I think this patch is good to go in.
> If not, then we'll have to wait to hear why he thinks the function is
> necessary.
I've already reviewed this patch once :-)
The inlined mi_arena already has 3-deep control structures and inlining
it makes them 4-deep control structures.
None of this code is in the hot path, so therefore my guiding principle
is to make this easier to debug and maintain.
The refactoring done in this patch doesn't meet my guiding principles,
thus I suggested we make it a distinct function. If later profiling
shows it's taking up too much time *then* we can optimize it by
reorganizing.
I'm opposed to a patch that makes the code more difficult to read.
If the goal is to remove nested functions, then do that, but do it simply
and make it easy to review.
Cheers,
Carlos.