Package: coreutils;
Reported by: Petros Aggelatos <petrosagg <at> gmail.com>
Date: Sun, 30 Jun 2013 05:23:01 UTC
Severity: normal
Done: Bernhard Voelker <mail <at> bernhard-voelker.de>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Pádraig Brady <P <at> draigBrady.com> To: 14752 <at> debbugs.gnu.org Subject: bug#14752: [PATCH] sort: print warning when fork() failed for --compress-program Date: Thu, 16 Jul 2015 04:09:53 +0100
On 16/07/15 03:00, Azat Khuzhin wrote: > On Wed, May 28, 2014 at 04:15:51PM +0100, Pádraig Brady wrote: >> On 05/26/2014 10:10 PM, Pádraig Brady wrote: >>> On 05/26/2014 10:00 PM, Azat Khuzhin wrote: >>>>> So the issue here is that sort is allocating >>>>> a large buffer up front thus impacting the fork(). >>>>> Really sort(1) should be trying to avoid this issue >>>>> in the first place, and the issue is already logged at: >>>>> http://bugs.gnu.org/14752 >>>> >>>> Yes this is the same as I linked above. >>>> Does any body have a patch for this, or should I start working on this? >>> >>> I was waiting for a patch that didn't materialize. >>> I'll have a look myself now. >> >> So I had a look and the change while definitely worth doing >> is a bit invasive and so probably not appropriate for the impending release, >> as that's focusing on bug fixes rather than performance characteristics. >> >> Some implementation notes for reference... > > Hi Pádraig, > > Thanks for your vision/thoughts about this problem, they are very > detailed. > >> vfork() is portable only when one essentially just does an >> execve() right after the vfork(). Therefore just for fire and forget processes. >> Anything where you need to interact with the sub process like setting up files >> to communicate etc. is going to have portability issues. Even using execvp() >> is problematic I understand. Also sort is multithreaded which might further >> complicate things. > > Agree. > >> Leveraging posix_spawn() is promising, as the main reason for that >> interface is to provide an efficient fork()+exec() mechanism. >> That can be implemented using vfork() or with clone() on Linux. >> The implementation in glibc is only a token one however as it >> just uses fork()+exec() usually. One can override with the >> POSIX_SPAWN_USEVFORK flag, but there are many non obvious >> implementation gotchas with doing that. Again being multithreaded >> may complicate things here. Note the musl, freebsd, osx and solaris >> posix_spawn() implementations are efficient, which would be >> another reason to use this assuming the glibc/gnulib implementation >> is fixed up. > > AFAIK posix_spawn() in glibc will be something like fork() if you need > to setup files, IOW it seems that sort can't posix_spawn() for > compress-prog to fix this problem: > > __spawni: > ... > if ((flags & POSIX_SPAWN_USEVFORK) != 0 > /* If no major work is done, allow using vfork. Note that we > might perform the path searching. But this would be done by > a call to execvp(), too, and such a call must be OK according > to POSIX. */ > || ((flags & (POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF > | POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER > | POSIX_SPAWN_SETPGROUP | POSIX_SPAWN_RESETIDS)) == 0 > && file_actions == NULL)) > new_pid = __vfork (); > else > new_pid = __fork (); > > While sort must use file_actions, no? > > Am I missing something? Right the glibc implementation currently doesn't benefit here. Though it's no worse, and other platforms benefit. There has been a little movement on the associated glibc bug since: https://sourceware.org/bugzilla/show_bug.cgi?id=10354 >> Another option would be for sort(1) to start up a helper child process, >> before we allocate much memory. Then we could communicate descriptors >> back and forth to that, and it could deal with forking the children. >> That would be portable too, but a little involved. Ideally we could >> keep the complication within posix_spawn() instead. > > So I guess that this is the only choice, and since there is no activity > since 2014, I have WIP/RFC patch that implements this, it passes some > basic tests, but not all (I'm working on it) and also needs in cleanup > and prettying, as for now maybe somebody have some comments? > > You could find patches in attachments and via git: > git <at> github.com:azat/coreutils.git sort-compress-prog-fixes-v2 Thanks for picking this up again. It is quite involved as expected. >> Note this is a general issue not just related to sort(1). >> Many servers for example whether written in java/python/ruby or whatever >> have this issue when they use lots of memory and would like to >> popen() something. So fixing up the glibc posix_spawn() implementation >> would be very useful so that popen() etc. within glibc and the >> various language runtimes could leverage. With the above general benefits in mind, and the complexity of the sort fork server implementation, I'm inclined to think working on posix_spawn() in glibc is more beneficial, and might actually be as easy. Also sort(1) can be moved to using posix_spawn() independently. >> >> Some links for reference: >> >> http://www.oracle.com/technetwork/server-storage/solaris10/subprocess-136439.html >> https://sourceware.org/ml/libc-help/2010-10/msg00001.html >> https://sourceware.org/bugzilla/show_bug.cgi?id=10354 >> https://sourceware.org/bugzilla/show_bug.cgi?id=378 >> http://git.musl-libc.org/cgit/musl/tree/src/process/posix_spawn.c >> http://ewontfix.com/7/ >> http://stackoverflow.com/questions/2731531/faster-forking-of-large-processes-on-linux >> http://stackoverflow.com/questions/8152076/spawn-process-from-multithreaded-application >> https://github.com/rtomayko/posix-spawn/blob/master/ext/posix-spawn.c >> http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/ >> http://code.google.com/p/popen-noshell/source/browse/trunk/popen_noshell.c thanks! Pádraig.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.