On 06/15/2011 09:40 AM, Ed W wrote: > On 15/06/2011 06:57, Timo Teräs wrote: >> Rabbit hole goes deep. It's the whole family of reading lines in libbb >> that's brain damaged this way. xmalloc_fget*. >> >> In the sources there's even faster versions, disabled for some reason. > > Sorry that I didn't manage to explain things more clearly - I got > somewhere near where you are now but there is some subtlety in figuring > out how to proceed. There appears to be some attempt to make the line > reader generic and split on nulls, \n, \\, etc - however, that appears > to have prevented several attempts at optimisation? > > Readline does seem very tempting for platforms where it's supported..?
You probably meant getline(). Also getdelim() is very nice to replace the NUL termination stuff. It seems that mostly the code wants either \n *or* \0 termination at given time. And the above extensions work perfectly for that. However, sed seems to depend on the behaviour that both \n *and* \0 work as line separators. Wonder if the sed code could be removed of that assumption. >> I also remember that there was issue with fgets, that it didn't detect >> NUL bytes properly, and was broken if the file has \0 and or \n as >> separator. > > Grep-ing on where the functions are used shows not so many places... > Where it is used it's used as several different synonmys, all of which > basically boil down to the same line reader... Yes. There's the xmalloc_fget* function calls, and the bb_get_chunk* stuff that is intertwined. Both have two fundamental problems: * uses getch, which does locking on per-char read * both malloc new memory on each call The first issue is probably bigger performance wise, and changing the code to use getline()/getdelim() will probably give similar performance improvement as to just using getc_unlocked(). Fixing the second problem is more intrusive. Most of the code doesn't load the whole file (except sort, and maybe some others); but instead handle it line-by-line. In those cases, it's really beneficial to cache the allocated line buffer so we don't do malloc/realloc/free all the time. >> It seems that the whole API of reading lines needs a rework, and it >> would speed up a bunch of bb tools. > > I think sed caught my eye as the main likely beneficiary? Grep also for "xmalloc_fget". That's used in many other applets too like: tar, cpio, dpkg, patch, grep and many others. Thinking more, those places might depend on the \0 *and* \n line termination detection too. > Thanks for looking into this further! Did you try to modprobe patch I posted? It should give considerable imporvement to modprobe too. - Timo _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
