Subject: Re: 25.0.50; [PATCH 0/7] Emacs can return too fast when reading from any processes
From: Paul Eggert <[email-masked]>
To: Ian Kelling <[email-masked]>
Cc: [email-masked]
Date: Sun, 05 Jul 2015 19:28:54 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the patches; they were very clear and organized, in a difficult code 
area.  I installed them into the Emacs master with minor changes, along with the 
attached further patch, and am marking this bug as done.

As far as patch format goes, I mildly prefer attachments but the form you used 
was fine.  My preference is because I am not on the bug-gnu-emacs mailing list, 
and don't know how to retrieve your email from debbugs.gnu.org or from 
lists.gnu.org -- the best I could come up with was the mbox format of 
lists.gnu.org but that did not apply cleanly.
From fea99d2d8563334a8c88ec4207064ce49d77dd1f Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 5 Jul 2015 19:19:13 -0700
Subject: [PATCH 8/8] Avoid duplicate calls to current_timespec

* src/process.c (wait_reading_process_output):
Cache current_timespec results as long as we're not waiting.
---
 src/process.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/process.c b/src/process.c
index 8a8dad7..9d8fa22 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4591,6 +4591,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   int got_some_output = -1;
   ptrdiff_t count = SPECPDL_INDEX ();
 
+  /* Close to the current time if known, an invalid timespec otherwise.  */
+  struct timespec now = invalid_timespec ();
+
   FD_ZERO (&Available);
   FD_ZERO (&Writeok);
 
@@ -4611,8 +4614,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   else if (time_limit > 0 || nsecs > 0)
     {
       wait = TIMEOUT;
-      end_time = timespec_add (current_timespec (),
-                               make_timespec (time_limit, nsecs));
+      now = current_timespec ();
+      end_time = timespec_add (now, make_timespec (time_limit, nsecs));
     }
   else
     wait = INFINITY;
@@ -4637,7 +4640,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       /* Exit if already run out.  */
       if (wait == TIMEOUT)
 	{
-	  struct timespec now = current_timespec ();
+	  if (!timespec_valid_p (now))
+	    now = current_timespec ();
 	  if (timespec_cmp (end_time, now) <= 0)
 	    break;
 	  timeout = timespec_sub (end_time, now);
@@ -4830,7 +4834,6 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	}
       else
 	{
-
 	  /* Set the timeout for adaptive read buffering if any
 	     process has non-zero read_output_skip and non-zero
 	     read_output_delay, and we are not reading output for a
@@ -4876,17 +4879,21 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	      && timespec_valid_p (timer_delay)
 	      && timespec_cmp (timer_delay, timeout) < 0)
 	    {
-	      struct timespec timeout_abs = timespec_add (current_timespec (),
-							  timeout);
+	      if (!timespec_valid_p (now))
+		now = current_timespec ();
+	      struct timespec timeout_abs = timespec_add (now, timeout);
 	      if (!timespec_valid_p (got_output_end_time)
-		  || timespec_cmp (timeout_abs,
-				   got_output_end_time) < 0)
+		  || timespec_cmp (timeout_abs, got_output_end_time) < 0)
 		got_output_end_time = timeout_abs;
 	      timeout = timer_delay;
 	    }
 	  else
 	    got_output_end_time = invalid_timespec ();
 
+	  /* NOW can become inaccurate if time can pass during pselect.  */
+	  if (timeout.tv_sec > 0 || timeout.tv_nsec > 0)
+	    now = invalid_timespec ();
+
 #if defined (HAVE_NS)
           nfds = ns_select
 #elif defined (HAVE_GLIB)
@@ -4965,14 +4972,21 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
              haven't lowered our timeout due to timers or SIGIO and
              have waited a long amount of time due to repeated
              timers.  */
-	  struct timespec now = current_timespec ();
-          if (wait < TIMEOUT
-	      || (wait == TIMEOUT && timespec_cmp (end_time, now) <= 0)
-	      || (!process_skipped && got_some_output > 0
-		  && (!timespec_valid_p (got_output_end_time)
-		      || timespec_cmp (got_output_end_time, now) <= 0)
-		  && (timeout.tv_sec > 0 || timeout.tv_nsec > 0)))
+	  if (wait < TIMEOUT)
 	    break;
+	  struct timespec cmp_time
+	    = (wait == TIMEOUT
+	       ? end_time
+	       : (!process_skipped && got_some_output > 0
+		  && (timeout.tv_sec > 0 || timeout.tv_nsec > 0))
+	       ? got_output_end_time
+	       : invalid_timespec ());
+	  if (timespec_valid_p (cmp_time))
+	    {
+	      now = current_timespec ();
+	      if (timespec_cmp (cmp_time, now) <= 0)
+		break;
+	    }
 	}
 
       if (nfds < 0)
-- 
2.1.0