[PATCH] debloat lash by 4.803725881571%

Rob Landley rob at landley.net
Mon Jun 19 01:21:01 UTC 2006


On Thursday 15 June 2006 5:03 pm, Bernhard Fischer wrote:
> Hi,
>
> The attached patch (which is only very lightly tested so far) does shave
> some bloat off lash:
>
>    text	   data	    bss	    dec	    hex	filename
>    7288	    168	     59	   7515	   1d5b	shell/lash.o.orig
>    6927	    168	     59	   7154	   1bf2	shell/lash.o

Nice.

> 1) I'm not entirely sure that i got the bb_skip_whitespace stuff right,
>    would be nice if someone could eye these for a second.
> 2) Also i'm not sure if we want to put this into the repo just now or if
>    we should wait for the 1.2 release-branch to be created and
>    eventually apply it afterwards. I'll leave the decision whether to
>    apply it for 1.2.0 or not to the maintainer..

Since there's about a 50% chance lash will go away during 1.3 (replaced by 
bbsh), might as well apply it now.

Do the ATTRIBUTE_UNUSEDS actually save space, or is this fighting off 
nonexistent compiler problems?  Because inside a function the compiler should 
darn well be able to figure this stuff out on its own without annotation.  
And if it's doing unit-at-a-time it should be able to figure this out for 
static functions.

Why do you do this sort of thing repeatedly?

-				while(isalnum(*src) || *src=='_') src++;
+				while((isalnum)(*src) || *src=='_') src++;

You add #defines inside functions, which is just gross:

+#define DONE (1)
+#define SAW_QUOTE (2)

And this points out a bug in libbb:
+	*command_ptr = (char*)bb_skip_whitespace(*command_ptr);

The bug is that skip_whitespace has its return type declared const.  It has NO 
BUSINESS making claims about what people who called it are going to do with 
the data...  Ok, dealt with that.

Huh.  I'm now about halfway through applying the lash cleanup, and the result 
is noticeably _bigger_...

make bloatcheck
function                                             old     new   delta
run_command                                          271     983    +712
builtin_export                                       373     366      -7
builtin_read                                         382     366     -16
builtin_source                                       229     204     -25
.rodata                                           186778  186746     -32
busy_loop                                           3857    3286    -571
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/5 up/down: 712/-651)           Total: 61 bytes

I have no idea why that is.  Attached is how far I got.  gcc 4.03 seems to 
have inlined busy_loop in run_command (even though it's also called from 
builtin_source...?)

I'm going to go back to banging on the mdev thing now...

Rob
-- 
Never bet against the cheap plastic solution.



More information about the busybox mailing list