2003-07-27 15:04:56

by Ren

[permalink] [raw]
Subject: [PATCH] Inline vfat_strnicmp()

Hi,

the function vfat_strnicmp() has just one callsite. Inlining it
actually shrinks vfat.o slightly.

Ren?



diff -u ./fs/vfat/namei.c~ ./fs/vfat/namei.c
--- ./fs/vfat/namei.c~ 2003-07-27 16:41:36.000000000 +0200
+++ ./fs/vfat/namei.c 2003-07-27 17:12:12.000000000 +0200
@@ -103,7 +103,7 @@
return nc ? nc : c;
}

-static int
+static inline int
vfat_strnicmp(struct nls_table *t, const unsigned char *s1,
const unsigned char *s2, int len)
{


2003-07-27 16:19:39

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

Ren <[email protected]> writes:

> the function vfat_strnicmp() has just one callsite. Inlining it
> actually shrinks vfat.o slightly.

Thanks. I'll submit this patch to Linus.
--
OGAWA Hirofumi <[email protected]>

2003-07-31 12:35:04

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

On 27 July 2003 19:33, OGAWA Hirofumi wrote:
> Ren <[email protected]> writes:
>
> > the function vfat_strnicmp() has just one callsite. Inlining it
> > actually shrinks vfat.o slightly.
>
> Thanks. I'll submit this patch to Linus.

Just to deinline it in some months?

Come on, automatically inlining static functions with
just one callsite is a compiler's job. Don't do it.
--
vda

2003-07-31 13:55:37

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

Denis Vlasenko <[email protected]> writes:

> On 27 July 2003 19:33, OGAWA Hirofumi wrote:
> > Ren <[email protected]> writes:
> >
> > > the function vfat_strnicmp() has just one callsite. Inlining it
> > > actually shrinks vfat.o slightly.
> >
> > Thanks. I'll submit this patch to Linus.
>
> Just to deinline it in some months?
>
> Come on, automatically inlining static functions with
> just one callsite is a compiler's job. Don't do it.

Unfortunately "gcc version 3.2.3 20030415 (Debian prerelease)"
doesn't, at least.
--
OGAWA Hirofumi <[email protected]>

2003-07-31 14:04:47

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

On Thu, Jul 31, 2003 at 10:52:23PM +0900, OGAWA Hirofumi wrote:
> Denis Vlasenko <[email protected]> writes:
>
> > On 27 July 2003 19:33, OGAWA Hirofumi wrote:
> > > Ren <[email protected]> writes:
> > >
> > > > the function vfat_strnicmp() has just one callsite. Inlining it
> > > > actually shrinks vfat.o slightly.
> > >
> > > Thanks. I'll submit this patch to Linus.
> >
> > Just to deinline it in some months?
> >
> > Come on, automatically inlining static functions with
> > just one callsite is a compiler's job. Don't do it.
>
> Unfortunately "gcc version 3.2.3 20030415 (Debian prerelease)"
> doesn't, at least.

And how big is the performance loss? Is it even measurable?
And even if it is, is optimizing this really worth the trouble?


Regards: David Weinehall
--
/) David Weinehall <[email protected]> /) Northern lights wander (\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\) http://www.acc.umu.se/~tao/ (/ Full colour fire (/

2003-07-31 14:18:57

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

On 31 July 2003 16:52, OGAWA Hirofumi wrote:
> Denis Vlasenko <[email protected]> writes:
>
> > On 27 July 2003 19:33, OGAWA Hirofumi wrote:
> > > Ren <[email protected]> writes:
> > >
> > > > the function vfat_strnicmp() has just one callsite. Inlining it
> > > > actually shrinks vfat.o slightly.
> > >
> > > Thanks. I'll submit this patch to Linus.
> >
> > Just to deinline it in some months?
> >
> > Come on, automatically inlining static functions with
> > just one callsite is a compiler's job. Don't do it.
>
> Unfortunately "gcc version 3.2.3 20030415 (Debian prerelease)"
> doesn't, at least.

Yes, but some future version would.

Since there is no substantial wins in hunting down
such statics, and there is some risk of code bloat when
big inlined statics get called from more that one callsite,
and it will be automatically handled by smarter compiler someday,
I think it makes perfect sense to avoid doing this.
--
vda

2003-07-31 15:02:00

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

David Weinehall <[email protected]> writes:

> And how big is the performance loss? Is it even measurable?
> And even if it is, is optimizing this really worth the trouble?

48bytes smaller than prev. I think It's not a clear reason for
rejecting a patch.
--
OGAWA Hirofumi <[email protected]>

2003-07-31 15:11:12

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

Denis Vlasenko <[email protected]> writes:

> Yes, but some future version would.
>
> Since there is no substantial wins in hunting down
> such statics, and there is some risk of code bloat when
> big inlined statics get called from more that one callsite,
> and it will be automatically handled by smarter compiler someday,
> I think it makes perfect sense to avoid doing this.

Could you tell me, if compiler does it in future? I'll gladly kill
that inline.
--
OGAWA Hirofumi <[email protected]>

2003-08-01 06:02:12

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

On 31 July 2003 18:07, OGAWA Hirofumi wrote:
> Denis Vlasenko <[email protected]> writes:
>
> > Yes, but some future version would.
> >
> > Since there is no substantial wins in hunting down
> > such statics, and there is some risk of code bloat when
> > big inlined statics get called from more that one callsite,
> > and it will be automatically handled by smarter compiler someday,
> > I think it makes perfect sense to avoid doing this.
>
> Could you tell me, if compiler does it in future? I'll gladly kill
> that inline.

I can't be 100.00% sure it will happen. I'd say 98.234235% ;)

Andrew Morton kills extra large inlines, and you are creating them :(
That's not ok. Just leave those poor static functions alone
until compiler will do them, all at once.
There are lots of other stuff to do in the kernel source.
--
vda

2003-08-01 15:48:58

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] Inline vfat_strnicmp()

Denis Vlasenko <[email protected]> writes:

> I can't be 100.00% sure it will happen. I'd say 98.234235% ;)
>
> Andrew Morton kills extra large inlines, and you are creating them :(
> That's not ok. Just leave those poor static functions alone
> until compiler will do them, all at once.
> There are lots of other stuff to do in the kernel source.

- That's smaller than prev in *real world*.
- You don't fix compiler.

End of story.
--
OGAWA Hirofumi <[email protected]>