[M3devel] Performance issues with Process.Wait under userthreads

[Mika Nystrom]

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