On Sun, 23 Jul 2023 00:36:09 +0200
"Roberto A. Foglietta" <roberto.foglie...@gmail.com> wrote:

> On Sat, 22 Jul 2023 at 21:29, tito <farmat...@tiscali.it> wrote:
> >
> > On Sat, 22 Jul 2023 19:31:28 +0200
> > "Roberto A. Foglietta" <roberto.foglie...@gmail.com> wrote:
> >
> > > On Sat, 22 Jul 2023 at 15:40, tito <farmat...@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!

I see but dropping the caches etc doesn't seem to be a typical use case either.

Using the same optimization flag -O3 the busybox applet in a real life 
system gives close empirical results, which is the results most
people in their normal life use cases (one shot, no loops running,
no files in memory, no dropped caches, no giant multi-GB files)
will see so the performance increase is swallowed by the system
or by other bottlenecks.

> > >
> > > 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.

I think the size will rather increase as there are a bunch of features
missing that the original bb implementation already has:

1) multiple file handling (a must i would dare to say)
2) -a -f -o -n -t command line options
The options are:
  -a - --all                Scan the entire file, not just the data section 
[default]
  -f --print-file-name      Print the name of the file before each string
  -n --bytes=[number]       Locate & print any NUL-terminated sequence of at
                                               least [number] characters 
(default 4).
  -t --radix={o,d,x}        Print the location of the string in base 8, 10 or 16
  -o                        An alias for --radix=o

3) output compatible with original gnu strings
 
> 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-

BTW: there still seem to be corner-cases:
list=`find /usr`
for i in $list; do if test -f $i; then  ./strings $i > out1.txt; strings $i > 
out2.txt; diff -q out1.txt out2.txt; fi; done
Files out1.txt and out2.txt differ
Files out1.txt and out2.txt differ
Files out1.txt and out2.txt differ
Files out1.txt and out2.txt differ

test is still running....

Ciao,
Tito
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to