public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Ken Brown <kbrown@cornell.edu>
To: cygwin-patches@cygwin.com
Subject: [PATCH 04/12] Cygwin: FIFO: reduce I/O interleaving
Date: Thu, 16 Jul 2020 12:19:07 -0400	[thread overview]
Message-ID: <20200716161915.16994-5-kbrown@cornell.edu> (raw)
In-Reply-To: <20200716161915.16994-1-kbrown@cornell.edu>

Add a bool member 'last_read' to the fifo_client_handler structure,
which is set to true on a successful read.  This is used by raw_read
as follows.

When raw_read is called, it first locates the writer (if any) for
which last_read is true.  raw_read tries to read from that writer and
returns if there is input available.  Otherwise, it proceeds to poll
all the writers, as before.

The effect of this is that if a writer writes some data that is only
partially read, the next attempt to read will continue to read from
the same writer.  This should reduce the interleaving of output from
different writers.
---
 winsup/cygwin/fhandler.h       |  3 +-
 winsup/cygwin/fhandler_fifo.cc | 55 +++++++++++++++++++++++++++++-----
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index b5bfdd0b3..221c856e6 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1298,7 +1298,8 @@ struct fifo_client_handler
 {
   HANDLE h;
   fifo_client_connect_state state;
-  fifo_client_handler () : h (NULL), state (fc_unknown) {}
+  bool last_read;  /* true if our last successful read was from this client. */
+  fifo_client_handler () : h (NULL), state (fc_unknown), last_read (false) {}
   void close () { NtClose (h); }
   fifo_client_connect_state set_state ();
 };
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 3685cc0c2..afe21a468 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -404,6 +404,7 @@ fhandler_fifo::update_my_handlers ()
 	  goto out;
 	}
       fc.state = shared_fc_handler[i].state;
+      fc.last_read = shared_fc_handler[i].last_read;
     }
 out:
   set_prev_owner (null_fr_id);
@@ -1200,15 +1201,56 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
       /* No one else can take ownership while we hold the reading_lock. */
       reading_lock ();
       take_ownership ();
-      /* Poll the connected clients for input. */
-      int nconnected = 0;
+      /* Poll the connected clients for input.  Make two passes.  On
+	 the first pass, just try to read from the client from which
+	 we last read successfully.  This should minimize
+	 interleaving of writes from different clients. */
       fifo_client_lock ();
+      /* First pass. */
+      int j;
+      for (j = 0; j < nhandlers; j++)
+	if (fc_handler[j].last_read)
+	  break;
+      if (j < nhandlers && fc_handler[j].state >= fc_closing)
+	{
+	  NTSTATUS status;
+	  IO_STATUS_BLOCK io;
+
+	  status = NtReadFile (fc_handler[j].h, NULL, NULL, NULL,
+			       &io, in_ptr, len, NULL, NULL);
+	  switch (status)
+	    {
+	    case STATUS_SUCCESS:
+	    case STATUS_BUFFER_OVERFLOW:
+	      /* io.Information is supposedly valid in latter case. */
+	      if (io.Information > 0)
+		{
+		  len = io.Information;
+		  fifo_client_unlock ();
+		  reading_unlock ();
+		  return;
+		}
+	      break;
+	    case STATUS_PIPE_EMPTY:
+	      break;
+	    case STATUS_PIPE_BROKEN:
+	      fc_handler[j].state = fc_disconnected;
+	      break;
+	    default:
+	      debug_printf ("NtReadFile status %y", status);
+	      fc_handler[j].state = fc_error;
+	      break;
+	    }
+	  fc_handler[j].last_read = false;
+	}
+
+      /* Second pass. */
+      int nconnected = 0;
       for (int i = 0; i < nhandlers; i++)
 	if (fc_handler[i].state >= fc_closing)
 	  {
 	    NTSTATUS status;
 	    IO_STATUS_BLOCK io;
-	    size_t nbytes = 0;
 
 	    nconnected++;
 	    status = NtReadFile (fc_handler[i].h, NULL, NULL, NULL,
@@ -1217,11 +1259,10 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
 	      {
 	      case STATUS_SUCCESS:
 	      case STATUS_BUFFER_OVERFLOW:
-		/* io.Information is supposedly valid. */
-		nbytes = io.Information;
-		if (nbytes > 0)
+		if (io.Information > 0)
 		  {
-		    len = nbytes;
+		    len = io.Information;
+		    fc_handler[i].last_read = true;
 		    fifo_client_unlock ();
 		    reading_unlock ();
 		    return;
-- 
2.27.0


  parent reply	other threads:[~2020-07-16 16:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 16:19 [PATCH 00/12] FIFO: fix multiple reader support Ken Brown
2020-07-16 16:19 ` [PATCH 01/12] Cygwin: FIFO: fix problems finding new owner Ken Brown
2020-07-16 16:19 ` [PATCH 02/12] Cygwin: FIFO: keep a writer count in shared memory Ken Brown
2020-07-16 16:19 ` [PATCH 03/12] Cygwin: fhandler_fifo::hit_eof: improve reliability Ken Brown
2020-07-16 16:19 ` Ken Brown [this message]
2020-07-16 16:19 ` [PATCH 05/12] Cygwin: FIFO: improve taking ownership in fifo_reader_thread Ken Brown
2020-07-16 16:19 ` [PATCH 06/12] Cygwin: FIFO: fix indentation Ken Brown
2020-07-16 16:19 ` [PATCH 07/12] Cygwin: FIFO: make certain errors non-fatal Ken Brown
2020-07-16 16:19 ` [PATCH 08/12] Cygwin: FIFO: add missing lock Ken Brown
2020-07-16 16:19 ` [PATCH 09/12] Cygwin: fhandler_fifo::take_ownership: don't set event unnecessarily Ken Brown
2020-07-16 16:19 ` [PATCH 10/12] Cygwin: FIFO: allow take_ownership to be interrupted Ken Brown
2020-07-16 16:19 ` [PATCH 11/12] Cygwin: FIFO: clean up Ken Brown
2020-07-16 16:19 ` [PATCH 12/12] Cygwin: FIFO: update commentary Ken Brown
2020-07-16 19:57 ` [PATCH 00/12] FIFO: fix multiple reader support Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200716161915.16994-5-kbrown@cornell.edu \
    --to=kbrown@cornell.edu \
    --cc=cygwin-patches@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).