Re: [PATCH] btrfs: make btrfs_node_key static inline

[David Sterba <dsterba@suse.cz>]

On Tue, Sep 14, 2021 at 10:08:36PM +0800, Su Yue wrote:
> 
> On Tue 14 Sep 2021 at 15:12, David Sterba <dsterba@suse.cz> wrote:
> 
> > On Tue, Sep 14, 2021 at 06:53:35PM +0800, Su Yue wrote:
> >> It looks strange that btrfs_node_key is in struct-funcs.c.
> >> So move it to ctree.h and make it static inline.
> >
> > "looks strange" is not a sufficient reason. Inlining a function 
> > means
> > that the body will be expanded at each call site, bloating the 
> > binary
> > code. Have you measured the impact of that?
> >
> Fair enough.
> 
> Before:
>    text    data     bss     dec     hex filename
> 1202418  123105   19384 1344907  14858b fs/btrfs/btrfs.ko
> After:
>    text    data     bss     dec     hex filename
> 1202620  123105   19384 1345109  148655 fs/btrfs/btrfs.ko
> 
> +202
> 
> > There's some performance cost of an non-inline function due to 
> > the call
> > overhead but it does not make sense to inline a function that's 
> > called
> > rarely and not in a tight loop. If you grep for the function 
> > you'd see
> > that it's called eg. once per function or in a loop that's not
> > performance critical on first sight (eg. in reada_for_search).
> 
> Right, the patch won't impact performance obviously at the cost of
> +202 binary size. So I'd drop the patch.

It does increase the size a bit but from what I've seen in the assembly
it's not that bad and still probably worth doing the inline. There's one
more extra call to read_extent_buffer (hidden behind read_eb_member
macro).

Cleaning up the node key helpers would be useful too, adding some
more helpers and not calling read_eb_member in the end. I have a WIP
patchset for that but had to leave it as there were bugs I did not find.
I can bounce it to you if you're interested.