[M3devel] Performance issues with Process.Wait under userthreads

[Tony Hosking]

This all seems pretty reasonable.

On Feb 12, 2011, at 3:43 PM, Mika Nystrom wrote:

> Hi again m3devel (especially Tony), 
> 
> I have finally taken a bite of a problem that has been annoying me
> for a very, very long time.
> 
> Under user threads, the code for Process.Wait is as follows (see 
> ThreadPosix.m3):
> 
> PROCEDURE WaitProcess (pid: int; VAR status: int): int =
>  (* ThreadPThread.m3 and ThreadPosix.m3 are very similar. *)
>  CONST Delay = 0.1D0;
>  BEGIN
>    LOOP
>      WITH r = Uexec.waitpid(pid, ADR(status), Uexec.WNOHANG) DO
>        IF r # 0 THEN RETURN r END;
>      END;
>      Pause(Delay);
>    END;
>  END WaitProcess;
> 
> It inserts a 0.1 second delay after each failed waitpid.  This is extremely
> annoying for programs that start a long sequence of child processes and
> wait for them in sequence.  Namely, the compiler itself.  As a result
> the cm3 compiler (and PM3's m3build) are normally very very slow when
> using user threads.  For about the last ten years, I've had a hacked
> up m3build (for my PM3 installation) that skips the Pause and busy-waits
> instead.  
> 
> Note there is another problem here.  Since the Modula-3 runtime ignores
> SIGCHLD, no zombie processes are created since the Unix system automatically
> reaps the child processes.  I can see this would be a problem since PIDs
> are eventually reused and ... couldn't Uexec.waitpid wind up referring to
> the wrong process??
> 
> I will further note that the comment in Process.i3 reads as follows:
> 
> PROCEDURE Wait(p: T): ExitCode;
> 
>    Wait until the process with handle p terminates, then free the operating system resources associated with the process and return an exit code indicating the reason for its termination. It is a checked runtime error to call Wait twice on the same process handle. 
> 
> I am going to take this as fair warning that Process.Create *may* use
> resources that are not going to be released until Process.Wait has been
> called.
> 
> I have modified (in my local copy of CM3) the system as follows.
> I have come up with a semi-general mechanism for immediately unblocking
> a thread on the receipt of a unix signal.  
> 
> 1. The system relies on changing
> ThreadPosix.XPause such that if a signal is allowed to wake up a threads,
> that fact is recorded in a new field in the thread's descriptor
> record (of type ThreadPosix.T).  
> 
> 2. On receipt of a waited-for unix signal, a mask is set and control
> is passed to the thread scheduler which maintains the non-zero mask for
> exactly one iteration through the thread ring.
> 
> 3. If a thread is paused and waiting for EITHER a signal or some time,
> the thread is released for running and the thread's waiting state is 
> cleared.
> 
> The changes are more or less as follows:
> 
> 1. I have added a new field of type "int" to ThreadPosix.T:
> 
>      (* if state = pausing, the time at which we can restart *)
>      waitingForTime: Time.T;
> 
> +     (* if state = pausing, the signal that truncates the pause *)
> +     waitingForSig: int := -1;
> +
>      (* true if we are waiting during an AlertWait or AlertJoin
>         or AlertPause *)
>      alertable: BOOLEAN := FALSE;
> 
> 
> 2. Modifications to pause:
> 
> + PROCEDURE SigPause(n: LONGREAL; sig: int)=
> +   <*FATAL Alerted*>
> +   VAR until := n + Time.Now ();
> +   BEGIN
> +     XPause(until, FALSE, sig);
> +   END SigPause;
> +
>  PROCEDURE AlertPause(n: LONGREAL) RAISES {Alerted}=
>    VAR until := n + Time.Now ();
>    BEGIN
>      XPause(until, TRUE);
>    END AlertPause;
> 
> ! PROCEDURE XPause (READONLY until: Time.T; alertable := FALSE; sig:int := -1)
> !   RAISES {Alerted} =
>    BEGIN
>      INC (inCritical);
>        self.waitingForTime := until;
>        self.alertable := alertable;
> +       IF sig # -1 THEN
> +         self.waitingForSig := sig
> +       END;
>        ICannotRun (State.pausing);
>      DEC (inCritical);
>      InternalYield ();
> 
> 3. The received-signals mask:
> 
> ! CONST MaxSigs = 64;
> ! TYPE Sig = [ 0..MaxSigs-1 ];
> !
> ! (* in order to listen to other signals, they have to be enabled in
> !    allow_sigvtalrm as well *)
> ! VAR (*CONST*) SIGCHLD := ValueOfSIGCHLD();
> !
> !     gotSigs := SET OF Sig { };
> !
> 
> ValueOfSIGCHLD() is a C function used to get the value of the SIGCHLD
> constant without guessing at it (in ThreadPosixC.c).
> 
> 4. changes to the signal handler:
> 
> ! PROCEDURE switch_thread (sig: int) RAISES {Alerted} =
>    BEGIN
>      allow_sigvtalrm ();
> !
> !     INC(inCritical);
> !     (* mark signal as being delivered *)
> !     IF sig >= 0 AND sig < MaxSigs THEN
> !       gotSigs := gotSigs + SET OF Sig { sig }
> !     END;
> !     DEC(inCritical);
> !
> !     IF inCritical = 0 AND heapState.inCritical = 0 THEN
> !       InternalYield ()
> !     END;
>    END switch_thread;
> 
> Note that I don't know if INC/DEC(inCritical) does exactly the right
> thing here.
> 
> 5. changes to the scheduler:
> 
>   a. thread wakeup
>              IF t.alertable AND t.alertPending THEN
>                CanRun (t);
>                EXIT;
> +
> +             ELSIF t.waitingForSig IN gotSigs THEN
> +               t.waitingForSig := -1;
> +               CanRun(t);
> +               EXIT;
> 
>              ELSIF t.waitingForTime <= now THEN
>                CanRun (t);
>                EXIT;
> !
> 
>   b. clearing the mask
> 
>        END;
>      END;
> 
> +     gotSigs := SET OF Sig {};
> +
>      IF t.state = State.alive AND (scanned OR NOT someBlocking) THEN
>        IF perfOn THEN PerfRunning (t.id); END;
>        (* At least one thread wants to run; transfer to it *)
> 
> 6. changes to WaitProcess (Process.Wait):
> 
>  PROCEDURE WaitProcess (pid: int; VAR status: int): int =
>    (* ThreadPThread.m3 and ThreadPosix.m3 are very similar. *)
> !   CONST Delay = 10.0D0;
>    BEGIN
>      LOOP
>        WITH r = Uexec.waitpid(pid, ADR(status), Uexec.WNOHANG) DO
>          IF r # 0 THEN RETURN r END;
>        END;
> !       SigPause(Delay,SIGCHLD);
>      END;
>    END WaitProcess;
> 
> 7. install signal handler even if program is single-threaded:
> 
>  BEGIN
> +   (* we need to call set up the signal handler for other reasons than
> +      just thread switching now *)
> +   setup_sigvtalrm (switch_thread);
>  END ThreadPosix.
> 
> 8. modify signal handler in ThreadPosixC.c to catch SIGCHLD:
> 
>    sigemptyset(&ThreadSwitchSignal);
>    sigaddset(&ThreadSwitchSignal, SIG_TIMESLICE);
> +   sigaddset(&ThreadSwitchSignal, SIGCHLD);
> 
>    act.sa_handler = handler;
>    act.sa_flags = SA_RESTART;
>    sigemptyset(&(act.sa_mask));
>    if (sigaction (SIG_TIMESLICE, &act, NULL)) abort();
> +   if (sigaction (SIGCHLD, &act, NULL)) abort();
> 
> I'll send the complete diff in a separate message for those who want
> to study it more closely.
> 
> I propose the above changes for inclusion in the current CM3 repository.
> 
>    Mika