Re: [PATCH] Inline useless nested function mi_arena.

["Carlos O'Donell" <carlos at 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.