[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <200906011657.20251.rgetz@blackfin.uclinux.org>
Date: Mon, 1 Jun 2009 16:57:20 -0400
From: Robin Getz <rgetz@...ckfin.uclinux.org>
To: "Matt Mackall" <mpm@...enic.com>
CC: "Sam Ravnborg" <sam@...nborg.org>,
"Bernhard Reutner-Fischer" <rep.dot.nop@...il.com>,
"Denis Vlasenko" <vda.linux@...glemail.com>,
"Rob Landley" <rob@...dley.net>, linux-kernel@...r.kernel.org,
Mike Frysinger <vapier@...too.org>
Subject: Re: [PATCH] update kernel's scripts/bloat-o-meter from busybox
On Mon 1 Jun 2009 15:48, Matt Mackall pondered:
> On Mon, 2009-06-01 at 15:26 -0400, Robin Getz wrote:
> > From: "Rob Landley" <rob@...dley.net>
> > From: "Denis Vlasenko" <vda.linux@...glemail.com>
> > From: "Bernhard Reutner-Fischer" <rep.dot.nop@...il.com>
> >
> > Since bloat-o-meter was added to the kernel's source tree, it has
> > received little attention - either it works really well - or no
> > one uses it. I suspect the first :)
> >
> > However - some folks who have been using it more (mainly as part
> > of busybox) have been poking at it more often - and the output is
> > a little more friendly now.
> >
> > http://git.busybox.net/busybox/log/scripts/bloat-o-meter
> >
> > I think Rob had been sending early patches both places, but
> > somewhere along the line things never made it to lkml.
>
> I probably dropped them on the ground during a busy spell.
Sorry - I didn't mean to sound like I was trying to blame anyone.
> > Acked-by: Robin Getz <rgetz@...ckfin.uclinux.org>
> >
> > ---
> > Since none of this is from me, I can only add my Acked by. Bernhard,
> > Denis, and Rob will need to add their Signed-off-by separately.
> >
> > ----
> >
> >
> > bloat-o-meter | 37 +++++++++++++++++++++++++++++--------
> > 1 file changed, 29 insertions(+), 8 deletions(-)
> >
> >
> >
> > Index: scripts/bloat-o-meter
> > ===================================================================
> > --- scripts/bloat-o-meter (revision 6437)
> > +++ scripts/bloat-o-meter (working copy)
> > @@ -9,18 +9,37 @@
> >
> > import sys, os, re
> >
> > -if len(sys.argv) != 3:
> > +def usage():
> > sys.stderr.write("usage: %s file1 file2\n" % sys.argv[0])
> > sys.exit(-1)
> >
> > +if len(sys.argv) < 3:
> > + usage()
> > +
> > +for f in sys.argv[1], sys.argv[2]:
>
> in sys.argv[1:3]: is a bit more standard
>
> But this test should instead happen inside getsizes, no loop needed.
>
> > + if not os.path.exists(f):
> > + sys.stderr.write("Error: file '%s' does not exist\n" % f)
> > + usage()
> > +
> > +nm_args = " ".join([x for x in sys.argv[3:]])
>
> nm_args = " ".join(sys.argv[3:])
>
> > def getsizes(file):
> > sym = {}
> > - for l in os.popen("nm --size-sort " + file).readlines():
> > - size, type, name = l[:-1].split()
> > - if type in "tTdDbB":
> > + for l in os.popen("nm --size-sort %s %s" % (nm_args, file)).readlines():
> > + l = l.strip()
> > + # Skip empty lines
> > + if not len(l): continue
>
> (seems to be some whitespace damage? there should be no tabs in this
> source)
That is the way it is in the busybox git tree (until Mike fixed it 10 minutes ago)
- I'll fix it on the next version I send...
> if not l
same.
> > + # Skip archive members
> > + if len(l.split()) == 1 and l.endswith(':'):
>
> if ' ' not in l ...
http://git.busybox.net/busybox/commit/?id=49bdf28c320bf4e02048eb0cbd115d84b8467cd9
OK with me - if it is OK with Bernhard.
> > + continue
> > + size, type, name = l.split()
> > + if type in "tTdDbBrR":
> > # function names begin with '.' on 64-bit powerpc
> > if "." in name[1:]: name = "static." + name.split(".")[0]
> > sym[name] = sym.get(name, 0) + int(size, 16)
> > + for l in os.popen("readelf -S " + file).readlines():
> > + x = l.split()
> > + if len(x)<6 or x[1] != ".rodata": continue
> > + sym[".rodata"] = int(x[5], 16)
>
> Not sure what this addition is about?
I think this is from Rob.
http://git.busybox.net/busybox/commit/?id=f14f7fc5cad5cac1a0913b85c8304f6fb77697ad
> > return sym
> >
> > old = getsizes(sys.argv[1])
> > @@ -53,8 +72,10 @@
> > delta.sort()
> > delta.reverse()
> >
> > -print "add/remove: %s/%s grow/shrink: %s/%s up/down: %s/%s (%s)" % \
> > - (add, remove, grow, shrink, up, -down, up-down)
> > -print "%-40s %7s %7s %+7s" % ("function", "old", "new", "delta")
> > +print "%-48s %7s %7s %+7s" % ("function", "old", "new", "delta")
> > for d, n in delta:
> > - if d: print "%-40s %7s %7s %+7d" % (n, old.get(n,"-"), new.get(n,"-"), d)
> > + if d: print "%-48s %7s %7s %+7d" % (n, old.get(n,"-"), new.get(n,"-"), d)
> > +print "-"*78
> > +total="(add/remove: %s/%s grow/shrink: %s/%s up/down: %s/%s)%%sTotal: %s
> > bytes"\
> > + % (add, remove, grow, shrink, up, -down, up-down)
> > +print total % (" "*(80-len(total)))
>
> Not terribly excited about this last bit, which is going out of its way
> to right-align the total? Who cares about that?
Rob did. :) Same patch as above.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists