Proposal for a new applet: strings

Sat Jul 22 22:36:09 UTC 2023

On Sat, 22 Jul 2023 at 21:29, tito <farmatito at tiscali.it> wrote:
>
> On Sat, 22 Jul 2023 19:31:28 +0200
> "Roberto A. Foglietta" <roberto.foglietta at gmail.com> wrote:
>
> > On Sat, 22 Jul 2023 at 15:40, tito <farmatito at tiscali.it> wrote:
> >
> > > Hi,
> > >
> > > I'm not the maintainer so I can say nothing about integration,
> > > I can just point out things that look strange to me and my limited knowledge.
> > > When I read that this code is faster vs other code as I'm a curious
> > > person I just try to see how much faster it is and why as there
> > > is always something to learn on the busybox mailing list.
> > > If in my little tests it is not faster then I think I'm entitled
> > > to ask questions about it as science results should be reproducible.
> > >
> > > For simple benchmarking maybe reading a big enough file
> > > into memory and feeding it to strings in a few 1000 iterations
> > > should do to avoid bias from hdd/sdd and system load, one shot shows:
> > >
> > > ramtmp="$(mktemp -p /dev/shm/)"
> > >  dd if=vmlinux.o of=$ramtmp
> > > echo $ramtmp
> > > /dev/shm/tmp.ll3G2kzKE1
> > >
> > > 1) coreutils strings
> > > time  strings $ramtmp > /dev/null
> >
> > This is not correct because you are reading a file in tmpfs while the
>
> Yes, this was exactly the purpose of the test to eliminate all
> factors connected to underlying block devices and time
> the speed of code of the different implementations.
>

Which is wrong because you did a hypothesis which is far away from the
typical usage and in some cases you can even use it because strings
over a 4GB ISO image would not necessarily fit into a tmpfs in every
system. Abstract benchmarks can be funny but do not depict/measure the
reality as usual. Extending this logic, we can trash the Ohm law
because we can reach in the laboratory a near zero temperature!

> >
> > Lines particularly long, more than 4096 characters are divided into
> > blocks with \n. It is clearly a corner case for which \n should be
>
> It was 35 corner cases in a handful of files due to a arbitrary hard-coded limitation.
> You should maybe run it on `find /`

Which can be solved quite easily with a cast of a bool:

bool pr = isPrintable(*ch);

use this bool instead of the check into the code and change this one

#define print_text(p,b,c) if(p-b >= 4) { *p++ = 0; printf("%s%c",b,c); }

print_text(p, buffer, pr ? *ch : '\n'); // print collected text

in such a way it prints the current char rather than the new line.
After this change the average of 2x speed has been maintained.

> >
> > > I suspect this could be a problem for integration  and also size of code after integration is relevant.
> >

The size can be reduced using two buffers only without losing
performances because three are redundant. Which is the current size of
the strings applet? Just to have an idea because the size of a single
binary with main() et company cannot immediately be compared with a
busybox applet. For sure is a lot more smaller than the strings which
also require a large shared library:

size /usr/bin/strings
   text    data     bss     dec     hex filename
  20943    1472      64   22479    57cf /usr/bin/strings

ldd /usr/bin/strings
linux-vdso.so.1 (0x00007ffde4fbc000)
libbfd-2.38-system.so => /lib/x86_64-linux-gnu/libbfd-2.38-system.so
(0x00007f6d64cf1000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f6d64ac9000)
libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f6d64aad000)
/lib64/ld-linux-x86-64.so.2 (0x00007f6d64e8c000)

size /lib/x86_64-linux-gnu/libbfd-2.38-system.so
   text    data     bss     dec     hex filename
1434786   94000     680 1529466 17567a
/lib/x86_64-linux-gnu/libbfd-2.38-system.so

> > It is a corner case that could be addressed. I did not check the size
> > of strings in busybox. However, once confirmed that the size is more
> > important than the speed for busybox - I agree on this - then it can
> > be proposed to binutils (or coreutils) depending on which package is
> > included. I found the binary version for aarch64 on binutils, AFAIR.
>
> I wonder why should they be wanting to change their stable code for
> a new implementation?

Because It is very easy to check that it works, it is 2x faster on
average and on a fine-tuned system can reach 4x, the size drops
dramatically considering to free the binary from the large shared
library.

In attachment the new version with the test suite and the benchmark
suite in the header. The benchmark suite did not change with respect
to the script file I just sent.

Best regards, R-
-------------- next part --------------
A non-text attachment was scrubbed...
Name: strings.c
Type: text/x-csrc
Size: 6018 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20230723/e2ed113e/attachment-0001.c>