[PATCH] shrink last_char_is function even more

[Tito]

On 7/9/20 9:13 PM, Tito wrote:
> On 7/9/20 9:56 PM, Martin Lewis wrote:
>> Please note that my original patch is still smaller:
>> http://lists.busybox.net/pipermail/busybox/2020-June/088026.html
> 
> Hi,
> yes I know. This is smaller than what is in git now.
> I understood that Denis rejected your patch:
> "This scans the string twice, unnecessarily. Let's not do that."
> Can't say If he will like mine.
>  
>> I'm not sure whether it's faster, it would be interesting to compare them.
> 
> Can you suggest how could this be achieved? a test program?

Hi,
by some rough test i did your version is way faster:

real    0m0.005s
user    0m0.006s
sys     0m0.002s

vs

real    0m0.095s
user    0m0.102s
sys     0m0.001s

This is on 10000 iterations on a 3000 char array.

Ciao,
Tito
 
> Ciao,
> Tito
> 
>> On Tue, 7 Jul 2020 at 14:17, Tito <farmatito at tiscali.it <mailto:farmatito at tiscali.it>> wrote:
>>
>>     Hi,
>>     attached you will find a patch that shrinks libbb's last_char_is function.
>>     bloatcheck is:
>>
>>     function                                             old     new   delta
>>     last_char_is                                          53      42     -11
>>     ------------------------------------------------------------------------------
>>     (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-11)             Total: -11 bytes
>>        text    data     bss     dec     hex filename
>>      981219   16907    1872  999998   f423e busybox_old
>>      981208   16907    1872  999987   f4233 busybox_unstripped
>>
>>     Patch for review:
>>
>>     --- libbb/last_char_is.c.orig   2020-07-05 09:54:24.737931000 +0200
>>     +++ libbb/last_char_is.c        2020-07-06 14:29:27.768898574 +0200
>>     @@ -11,14 +11,7 @@
>>      /* Find out if the last character of a string matches the one given */
>>      char* FAST_FUNC last_char_is(const char *s, int c)
>>      {
>>     -       if (s) {
>>     -               size_t sz = strlen(s);
>>     -               /* Don't underrun the buffer if the string length is 0 */
>>     -               if (sz != 0) {
>>     -                       s += sz - 1;
>>     -                       if ((unsigned char)*s == c)
>>     -                               return (char*)s;
>>     -               }
>>     -       }
>>     -       return NULL;
>>     -}
>>     +       if (!s || !*s) return NULL;
>>     +    while (*(s + 1)) s++;
>>     +       return (*s == c) ? (char *) s : NULL;
>>     +}
>>
>>
>>     The alternative version:
>>
>>     char* FAST_FUNC last_char_is(const char *s, int c)
>>     {
>>           if (!s || !*s) return NULL;
>>           while (*(++s));
>>           return (*(--s) == c) ? (char *)s : NULL;
>>     }
>>
>>     that was also posted to the list on my system was bigger:
>>
>>     function                                             old     new   delta
>>     last_char_is                                          53      46      -7
>>     ------------------------------------------------------------------------------
>>     (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-7)               Total: -7 bytes
>>
>>     Ciao,
>>     Tito
>>
>>
>>     P.S.: test program, if you are aware of other corner cases please tell me:
>>     ____________________________________________________________________________
>>
>>     char* last_char_is(const char *s, int c)
>>     {
>>             if (!s || !*s)
>>                     return NULL;
>>         while (*(s + 1))
>>                     s++;
>>             return (*s == c) ? (char *) s : NULL;
>>     }
>>
>>     int main (int argc, char **argv) {
>>             char *c;
>>             int ret = 0;
>>
>>             printf("Test 1 'prova','a' : ");
>>             c = last_char_is("prova", 'a');
>>             if (c !=  NULL && *c == 'a') {
>>                     puts("PASSED");
>>             } else {
>>                     puts("FAILED");
>>                     ret |= 1;
>>             }
>>             printf("Test 2 ''     ,'x' : ");
>>             c = last_char_is("", 'x');
>>             if (c != NULL) {
>>                     puts("FAILED");
>>                     ret |= 1;
>>
>>             } else {
>>                     puts("PASSED");
>>             }
>>             printf("Test 3  NULL  ,'3' : ");
>>             c = last_char_is(NULL, 'e');
>>             if (c != NULL) {
>>                     puts("FAILED");
>>                     ret |= 1;
>>             } else {
>>                     puts("PASSED");
>>             }
>>             printf("Test 4 'prova','x' : ");
>>             c = last_char_is("prova", 'x');
>>             if (c != NULL) {
>>                     puts("FAILED");
>>                     ret |= 1;
>>             } else {
>>                     puts("PASSED");
>>             }
>>             printf("Test 5 'prova','\\n': ");
>>             c = last_char_is("prova", '\n');
>>             if (c != NULL) {
>>                     puts("FAILED");
>>                     ret |= 1;
>>             } else {
>>                     puts("PASSED");
>>             }
>>             printf("Test 6 'prova','\\0': ");
>>             c = last_char_is("prova", 0);
>>             if (c != NULL) {
>>                     puts("FAILED");
>>                     ret |= 1;
>>             } else {
>>                     puts("PASSED");
>>             }
>>             return ret;
>>     }
>>     ____________________________________________________________________________________________
>>     _______________________________________________
>>     busybox mailing list
>>     busybox at busybox.net <mailto:busybox at busybox.net>
>>     http://lists.busybox.net/mailman/listinfo/busybox
>>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
>