Switch to side-by-side view

--- a/website/idxthreads/forkingRecoll.txt
+++ b/website/idxthreads/forkingRecoll.txt
@@ -19,39 +19,40 @@
 out that not so many were accurate and that a lot of questions were left as
 an exercise to the reader.
 
-This document will list the references I found reliable and interesting and
-describe the solution chosen along the other possible approaches.
-
 == Issues with fork
 
 The traditional way for a Unix process to start another is the
-fork()/exec() system call pair. The initial fork() duplicates the address
-space and resources (open files etc.) of the first process, then duplicates
-the thread of execution, ending up with 2 mostly identical processes.
-exec() then replaces part of the newly executing process with an address space
-initialized from an executable file, inheriting some of the old assets
++fork()+/+exec()+ system call pair. 
+
++fork()+ duplicates the process address space and resources (open files
+etc.), then duplicates the thread of execution, ending up with 2 mostly
+identical processes.  
+
++exec()+ then replaces part of the newly executing process with an address
+space initialized from an executable file, inheriting some of the resources
 under various conditions.
 
-As processes became bigger the copying-before-discard operation wasted
+As processes became bigger the copy-before-discard operation wasted
 significant resources, and was optimized using two methods (at very
 different points in time):
 
- - The first approach was to supplement fork() with the vfork() call, which
+ - The first approach was to supplement +fork()+ with the +vfork()+ call, which
    is similar but does not duplicate the address space: the new process
    thread executes in the old address space. The old thread is blocked
-   until the new one calls exec() and frees up access to the memory
+   until the new one calls +exec()+ and frees up access to the memory
    space. Any modification performed by the child thread persists when
    the old one resumes.
 
- - The more modern approach, which cohexists with vfork(), was to replace
+ - The more modern approach, which cohexists with +vfork()+, was to replace
    the full duplication of the memory space with duplication of the page
    descriptors only. The pages in the new process are marked copy-on-write
    so that the new process has write access to its memory without
-   disturbing its parent. The problem with this approach is that the
-   operation can still be a significant resource consumer for big processes
-   mapping a lot of memory. Many processes can fall in this category not
-   because they have huge data segments, but just because they are linked
-   to many shared libraries.
+   disturbing its parent. This approach was supposed to make +vfork()+
+   obsolete, but the operation can still be a significant resource consumer
+   for big processes mapping a lot of memory, so that +vfork()+ is still
+   around. Programs can have big memory spaces not only because they have
+   huge data segments (rare), but just because they are linked to many
+   shared libraries (more common).
 
 NOTE: Orders of magnitude: a *recollindex* process will easily grow into a
 few hundred of megabytes of virtual space. It executes the small and
@@ -60,7 +61,7 @@
 doing `fork()`/`exec()` housekeeping instead of useful work (this is on Linux,
 where `fork()` uses copy-on-write).
 
-Apart from the performance cost, another issue with fork() is that a big
+Apart from the performance cost, another issue with +fork()+ is that a big
 process can fail executing a small command because of the temporary need to
 allocate twice its address space. This is a much discussed subject which we
 will leave aside because it generally does not concern *recollindex*, which
@@ -68,16 +69,16 @@
 so that a temporary doubling is not an issue.
 
 The Recoll indexer is multithreaded, which may introduce other issues. Here
-is what happens to threads during the fork()/exec() interval:
-
- - fork():
+is what happens to threads during the +fork()+/+exec()+ interval:
+
+ - +fork()+:
    * The parent process threads all go on their merry way.
    * The child process is created with only one thread active, duplicated
-     from the one which called fork()
- - vfork()
-   * The parent process thread calling vfork() is suspended, the others
+     from the one which called +fork()+
+ - +vfork()+
+   * The parent process thread calling +vfork()+ is suspended, the others
      are unaffected.
-   * The child is created with only one thread, as for fork(). 
+   * The child is created with only one thread, as for +fork()+. 
      This thread shares the memory space with the parent ones, without
      having any means to synchronize with them (pthread locks are not
      supposed to work across processes): caution needed !
@@ -92,14 +93,14 @@
 at both ends which will prevents seeing EOFs etc.). Thanks to StackExchange
 user Celada for explaining this to me.
 
-For multithreaded programs, both fork() and vfork() introduce possibilities
+For multithreaded programs, both +fork()+ and +vfork()+ introduce possibilities
 of deadlock, because the resources held by a non-forking thread in the
 parent process can't be released in the child because the thread is not
 duplicated. This used to happen from time to time in *recollindex* because
-of an error logging call performed if the exec() failed after the fork()
+of an error logging call performed if the +exec()+ failed after the +fork()+
 (e.g. command not found).
 
-With vfork() it is also possible to trigger a deadlock in the parent by
+With +vfork()+ it is also possible to trigger a deadlock in the parent by
 (inadvertently) modifying data in the child. This could happen just
 link:http://www.oracle.com/technetwork/server-storage/solaris10/subprocess-136439.html[because
 of dynamic linker operation] (which, seriously, should be considered a
@@ -110,7 +111,7 @@
 snapshot of what it was in the parent, and the official word about what you
 can do is that you can only call
 link:http://man7.org/linux/man-pages/man7/signal.7.html[async-safe library
-functions] between 'fork()' and 'exec()'. These are functions which are
+functions] between +fork()+ and +exec()+. These are functions which are
 safe to call from a signal handler because they are either reentrant or
 can't be interrupted by a signal. A notable missing entry in the list is
 `malloc()`.
@@ -120,8 +121,8 @@
 logging call issue...).
 
 One of the approaches often proposed for working around this mine-field is
-to use an auxiliary, small, process to execute any command needed by the
-main one. The small process can just use fork() with no performance
+to use an auxiliary small process to execute any command needed by the main
+one. The small process can just use +fork()+/+exec()+ with no performance
 issues. This has the inconvenient of complicating communication a lot if
 data needs to be transferred one way or another.
 
@@ -164,28 +165,54 @@
 available on Solaris and quite necessary in fact, because we have no way to
 be sure that all open descriptors have the CLOEXEC flag set.
 
-12500 small .doc files:
-
-fork:  real 0m46.025s user 0m26.574s sys 0m39.494s
-vfork: real 0m18.223s user 0m17.753s sys 0m1.736s
-spawn/fork: real 0m45.726s user 0m27.082s sys 0m40.575s
-spawn/vfork: real 0m18.915s user 0m18.681s sys 0m3.828s
-
-No surprise here, given the implementation of posix_spawn(), it gets the
-same times as the fork/vfork options.
-
-It is difficult to ignore the 60% reduction in execution time offered by
-using 'vfork()'.
-
+So, no `posix_spawn()` for us (support was implemented inside
+*recollindex*, but the code is normally not used).
+
+== The chosen solution
+
+The previous version of +recollindex+ used to use +vfork()+ if it was running
+a single thread, and +fork()+ if it ran multiple ones.
+
+After another careful look at the code, I could see few issues with
+using +vfork()+ in the multithreaded indexer, so this was committed. 
+
+The only change necessary was to get rid on an implementation of the
+lacking Linux +closefrom()+ call (used to close all open descriptors above a
+given value). The previous Recoll implementation listed the +/proc/self/fd+
+directory to look for open descriptors but this was unsafe because of of
+possible memory allocations in +opendir()+ etc.
+
+== Test results
+
+.Indexing 12500 small .doc files 
+[options="header"]
+|===============================
+|call  |real      |user       |sys
+|fork  |0m46.025s |0m26.574s |0m39.494s
+|vfork |0m18.223s |0m17.753s |0m1.736s
+|spawn/fork| 0m45.726s|0m27.082s| 0m40.575s
+|spawn/vfork|0m18.915s|0m18.681s|0m3.828s
+|recoll 1.18|1m47.589s|0m21.537s|0m29.458s
+|================================
+
+No surprise here, given the implementation of +posix_spawn()+, it gets the
+same times as the +fork()+/+vfork()+ options.
+
+The tests were performed on an Intel Core i5 750 (4 cores, 4 threads).
+
+The last line is just for the fun: *recollindex* 1.18 (single-threaded)
+needed almost 6 times as long to process the same files... 
+
+It would be painful to play it safe and discard the 60% reduction in
+execution time offered by using +vfork()+.
+
+To this day, no problems were discovered, but, still crossing fingers...
+
+////
 Objections to vfork: 
-  ld.so locks
   sigaction locks
-
 https://bugzilla.redhat.com/show_bug.cgi?id=193631
-
 Is Linux vfork thread-safe ? Quoting interesting comments from Solaris
-implementation: 
-No answer to the issues cited though.
-
+implementation: No answer to the issues cited though.
 https://sourceware.org/bugzilla/show_bug.cgi?id=378
-Use vfork() in posix_spawn()
+////