Re: [PATCH] Inline useless nested function mi_arena.

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Tue, May 20, 2014 at 05:28:31PM -0400, Carlos O'Donell wrote:
> 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.

We agree on the guiding principle, it's just that our opinions on what
is easier to maintain and debug is different.

I prefer the inlining to a separate function for three reasons:

1. The separate function is called only once, so inlining does not
   result in duplication of code.

2. The 4-deep nesting does not make the code unreadable, because the
   code flow otherwise is quite simple.  There are no arbitrary jumps
   out of loops and the conditions are very simple.

3. Making a separate non-inline function would require passing of a
   lot of context to the function for just one call - you would end up
   with a function call with 10 arguments.  That IMO is much more
   unwieldy to maintain than the simple inline bit.

That said, the best option for what I am planning to do with this
function (add mmap stats and add an option to produce JSON) would be
to collate all the statistics into a struct and pass to struct to a
printer function when we're done collecting data.  However, that is
even more code rework, so it might be suitable for a separate pass.

Siddhesh

Attachment: pgp5U2fkIhkgr.pgp
Description: PGP signature