[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 1/1] libbb: reduce the overhead of single parameter bb_error_msg() calls
From:       James Byrne <james.byrne () origamienergy ! com>
Date:       2018-05-27 11:41:47
Message-ID: ccc1ef8c-8dd3-5e75-abdd-03bdb2faf756 () origamienergy ! com
[Download RAW message or body]

Hi Denys,

On 26/05/18 17:21, Denys Vlasenko wrote:
> The patch is whitespace damaged, please send as attachment next time.

I sent with 'git send-email' as I thought that would avoid any damage,
but clearly it didn't work. Will send as an attachment next time.

> On Fri, May 11, 2018 at 7:32 PM, James Byrne
> <james.byrne@origamienergy.com> wrote:
> > The space saving gained by this (using 'defconfig' on x86_64 with gcc
> > Ubuntu 6.4.0-17ubuntu1~16.04) was:
> > 
> > ------------------------------------------------------------------------------
> > (add/remove: 3/0 grow/shrink: 4/301 up/down: 73/-910)        Total: -837
> > bytes
> > text    data     bss     dec     hex filename
> > 936949    4263    1856  943068   e63dc busybox_old
> > 936098    4263    1856  942217   e6089 busybox_unstripped
> 
> Can't reproduce:
> 
> function                                             old     new   delta
> bb_simple_herror_msg                                   -      14     +14
> bb_simple_error_msg                                    -      14     +14
> bb_simple_herror_msg_and_die                           -      11     +11
> bb_simple_error_msg_and_die                            -      11     +11
> sha1_process_block64                                 322     328      +6
> expand_one_var                                      1599    1603      +4
> fail_hunk                                            136     139      +3
> mklocal                                              290     288      -2
> xgethostbyname                                        27      23      -4
> find_applet_by_name                                  128     124      -4
> die                                                   35      31      -4
> chat_main                                           1379    1375      -4
> arp_show                                             704     700      -4
> arp_main                                            1455    1451      -4
> add_interface                                        103      99      -4
> acpid_main                                          1188    1184      -4
> wget_main                                           2389    2384      -5
> tftp_protocol                                       1839    1832      -7
> getoptscmd                                           595     587      -8
> builtin_read                                         210     202      -8
> nfsmount                                            3556    3546     -10
> ------------------------------------------------------------------------------
> (add/remove: 4/0 grow/shrink: 3/14 up/down: 63/-72)            Total: -9 bytes

That's interesting. I tried 'defconfig' on x86_64 with GCC 6, 7 and 8
(using glibc) and got much larger savings each time. What compiler and
config were you testing?

> > +/* Override bb_error_msg() and related functions with macros that will
> > + * substitute them for the equivalent bb_simple_error_msg() function when
> > + * they are used with only a single parameter. Macro approach inspired by
> > + * https://gustedt.wordpress.com/2010/06/08/detect-empty-macro-arguments and
> > + * https://gustedt.wordpress.com/2010/06/03/default-arguments-for-c99
> > + */
> > +#define _ARG16(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, \
> > _15, ...) _15 +#define BB_MSG_KIND(...) _ARG16(__VA_ARGS__, , , , , , , , , , , , \
> > , , , _simple) +#define _BB_MSG(name, kind, ...) bb##kind##name(__VA_ARGS__)
> > +#define BB_MSG(name, kind, ...) _BB_MSG(name, kind, __VA_ARGS__)
> > +#define bb_error_msg(...) BB_MSG(_error_msg, BB_MSG_KIND(__VA_ARGS__), \
> > __VA_ARGS__) +#define bb_error_msg_and_die(...) BB_MSG(_error_msg_and_die, \
> > BB_MSG_KIND(__VA_ARGS__), __VA_ARGS__) +#define bb_perror_msg(...) \
> > BB_MSG(_perror_msg, BB_MSG_KIND(__VA_ARGS__), __VA_ARGS__) +#define \
> > bb_perror_msg_and_die(...) BB_MSG(_perror_msg_and_die, BB_MSG_KIND(__VA_ARGS__), \
> > __VA_ARGS__)
> 
> NAK this part. I prefer explicit use of _simple_ functions,
> not this magic conversion.

I'm happy to resubmit with everything changed to explicit _simple_
functions. While I tend to agree that less magic is better, the main
disadvantage will be that over time people will keep adding new single
parameter calls to bb_error_msg() etc., and they will need periodic
manual replacement, whereas the magic macros do it automatically.

Regards,

James
The contents of this email and any attachment are confidential to the intended \
recipient(s). If you are not an intended recipient: (i) do not use, disclose, \
distribute, copy or publish this email or its contents; (ii) please contact the \
sender immediately; and (iii) delete this email. Our privacy policy is available \
here: https://origamienergy.com/privacy-policy/. Origami Energy Limited (company \
number 8619644); Origami Storage Limited (company number 10436515) and OSSPV001 \
Limited (company number 10933403), each registered in England and each with a \
registered office at: Ashcombe Court, Woolsack Way, Godalming, GU7 1LQ. \
_______________________________________________ busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]