user/dev discussion of public-inbox itself
 help / color / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 07/57] ds: split out from ->flush_write and ->write
Date: Mon, 24 Jun 2019 02:52:08 +0000
Message-ID: <20190624025258.25592-8-e@80x24.org> (raw)
In-Reply-To: <20190624025258.25592-1-e@80x24.org>

Get rid of the confusing $need_queue variable and all
the associated documentation for it.  Instead, make it
obvious that we're either skipping the write buffer or
flushing the write buffer by splitting the sub in two.
---
 lib/PublicInbox/DS.pm | 141 +++++++++++++++++-------------------------
 1 file changed, 58 insertions(+), 83 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 154fd4dd..f1b7bab7 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -480,6 +480,42 @@ sub close {
     return 0;
 }
 
+# returns 1 if done, 0 if incomplete
+sub flush_write ($) {
+    my ($self) = @_;
+    my $sock = $self->{sock} or return 1;
+    my $wbuf = $self->{wbuf};
+
+    while (my $bref = $wbuf->[0]) {
+        my $ref = ref($bref);
+        if ($ref eq 'SCALAR') {
+            my $len = bytes::length($$bref);
+            my $off = $self->{wbuf_off} || 0;
+            my $to_write = $len - $off;
+            my $written = syswrite($sock, $$bref, $to_write, $off);
+            if (defined $written) {
+                if ($written == $to_write) {
+                    shift @$wbuf;
+                } else {
+                    $self->{wbuf_off} = $off + $written;
+                }
+                next; # keep going until EAGAIN
+            } elsif ($! == EAGAIN) {
+                $self->watch_write(1);
+            } else {
+                $self->close;
+            }
+            return 0;
+        } else { #($ref eq 'CODE') {
+            shift @$wbuf;
+            $bref->();
+        }
+    } # while @$wbuf
+
+    $self->watch_write(0);
+    1; # all done
+}
+
 =head2 C<< $obj->write( $data ) >>
 
 Write the specified data to the underlying handle.  I<data> may be scalar,
@@ -489,9 +525,8 @@ it returns 1, caller should stop waiting for 'writable' events)
 
 =cut
 sub write {
-    my PublicInbox::DS $self;
-    my $data;
-    ($self, $data) = @_;
+    my ($self, $data) = @_;
+    return flush_write($self) unless defined $data;
 
     # nobody should be writing to closed sockets, but caller code can
     # do two writes within an event, have the first fail and
@@ -501,91 +536,31 @@ sub write {
     # just lie and say it worked.  it'll be dead soon and won't be
     # hurt by this lie.
     my $sock = $self->{sock} or return 1;
-
-    my $bref;
-
-    # just queue data if there's already a wait
-    my $need_queue;
+    my $ref = ref $data;
+    my $bref = $ref ? $data : \$data;
     my $wbuf = $self->{wbuf};
+    if (@$wbuf) { # already buffering, can't write more...
+        push @$wbuf, $bref;
+        return 0;
+    } elsif ($ref eq 'CODE') {
+        $bref->();
+        return 1;
+    } else {
+        my $to_write = bytes::length($$bref);
+        my $written = syswrite($sock, $$bref, $to_write);
 
-    if (defined $data) {
-        $bref = ref $data ? $data : \$data;
-        if (scalar @$wbuf) {
+        if (defined $written) {
+            return 1 if $written == $to_write;
+            $self->{wbuf_off} = $written;
+            push @$wbuf, $bref;
+            return flush_write($self); # try until EAGAIN
+        } elsif ($! == EAGAIN) {
             push @$wbuf, $bref;
-            return 0;
-        }
-
-        # this flag says we're bypassing the queue system, knowing we're the
-        # only outstanding write, and hoping we don't ever need to use it.
-        # if so later, though, we'll need to queue
-        $need_queue = 1;
-    }
-
-  WRITE:
-    while (1) {
-        return 1 unless $bref ||= $wbuf->[0];
-
-        my $len;
-        eval {
-            $len = length($$bref); # this will die if $bref is a code ref, caught below
-        };
-        if ($@) {
-            if (UNIVERSAL::isa($bref, "CODE")) {
-                unless ($need_queue) {
-                    shift @$wbuf;
-                }
-                $bref->();
-
-                # code refs are just run and never get reenqueued
-                # (they're one-shot), so turn off the flag indicating the
-                # outstanding data needs queueing.
-                $need_queue = 0;
-
-                undef $bref;
-                next WRITE;
-            }
-            die "Write error: $@ <$bref>";
-        }
-
-        my $off = $self->{wbuf_off} // 0;
-        my $to_write = $len - $off;
-        my $written = syswrite($sock, $$bref, $to_write, $off);
-
-        if (! defined $written) {
-            if ($! == EAGAIN) {
-                # since connection has stuff to write, it should now be
-                # interested in pending writes:
-                if ($need_queue) {
-                    push @$wbuf, $bref;
-                }
-                $self->watch_write(1);
-                return 0;
-            }
-
-            return $self->close;
-        } elsif ($written != $to_write) {
-            if ($need_queue) {
-                push @$wbuf, $bref;
-            }
-            # since connection has stuff to write, it should now be
-            # interested in pending writes:
-            $self->{wbuf_off} = $off + $written;
             $self->watch_write(1);
-            return 0;
-        } elsif ($written == $to_write) {
-            delete $self->{wbuf_off};
-            $self->watch_write(0);
-
-            # this was our only write, so we can return immediately
-            # since we avoided incrementing the buffer size or
-            # putting it in the buffer.  we also know there
-            # can't be anything else to write.
-            return 1 if $need_queue;
-
-            shift @$wbuf;
-            undef $bref;
-            next WRITE;
+        } else {
+            $self->close;
         }
+        return 0;
     }
 }
 
-- 
EW


  parent reply index

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-24  2:52 [PATCH 00/57] ds: shrink, TLS support, buffer writes to FS Eric Wong
2019-06-24  2:52 ` [PATCH 01/57] ds: get rid of {closed} field Eric Wong
2019-06-24  2:52 ` [PATCH 02/57] ds: get rid of more unused debug instance methods Eric Wong
2019-06-24  2:52 ` [PATCH 03/57] ds: use and export monotonic now() Eric Wong
2019-06-24  2:52 ` [PATCH 04/57] AddTimer: avoid clock_gettime for the '0' case Eric Wong
2019-06-24  2:52 ` [PATCH 05/57] ds: get rid of on_incomplete_write wrapper Eric Wong
2019-06-24  2:52 ` [PATCH 06/57] ds: lazy initialize wbuf_off Eric Wong
2019-06-24  2:52 ` Eric Wong [this message]
2019-06-24  2:52 ` [PATCH 08/57] ds: lazy-initialize wbuf Eric Wong
2019-06-24  2:52 ` [PATCH 09/57] ds: don't pass `events' arg to EPOLL_CTL_DEL Eric Wong
2019-06-24  2:52 ` [PATCH 10/57] ds: remove support for DS->write(undef) Eric Wong
2019-06-24  2:52 ` [PATCH 11/57] http: favor DS->write(strref) when reasonable Eric Wong
2019-06-24  2:52 ` [PATCH 12/57] ds: share send(..., MSG_MORE) logic Eric Wong
2019-06-24  2:52 ` [PATCH 13/57] ds: switch write buffering to use a tempfile Eric Wong
2019-06-24  2:52 ` [PATCH 14/57] ds: get rid of redundant and unnecessary POLL* constants Eric Wong
2019-06-24  2:52 ` [PATCH 15/57] syscall: get rid of unused EPOLL* constants Eric Wong
2019-06-24  2:52 ` [PATCH 16/57] syscall: get rid of unnecessary uname local vars Eric Wong
2019-06-24  2:52 ` [PATCH 17/57] ds: set event flags directly at initialization Eric Wong
2019-06-24  2:52 ` [PATCH 18/57] ds: import IO::KQueue namespace Eric Wong
2019-06-24  2:52 ` [PATCH 19/57] ds: share watch_chg between watch_read/watch_write Eric Wong
2019-06-24  2:52 ` [PATCH 20/57] ds: remove IO::Poll support (for now) Eric Wong
2019-06-24  2:52 ` [PATCH 21/57] ds: get rid of event_watch field Eric Wong
2019-06-24  2:52 ` [PATCH 22/57] httpd/async: remove EINTR check Eric Wong
2019-06-24  2:52 ` [PATCH 23/57] spawn: remove `Blocking' flag handling Eric Wong
2019-06-24  2:52 ` [PATCH 24/57] qspawn: describe where `$rpipe' come from Eric Wong
2019-06-24  2:52 ` [PATCH 25/57] http|nntp: favor "$! == EFOO" over $!{EFOO} checks Eric Wong
2019-06-24  2:52 ` [PATCH 26/57] ds: favor `delete' over assigning fields to `undef' Eric Wong
2019-06-24  2:52 ` [PATCH 27/57] http: don't pass extra args to PublicInbox::DS::close Eric Wong
2019-06-24  2:52 ` [PATCH 28/57] ds: pass $self to code references Eric Wong
2019-06-24  2:52 ` [PATCH 29/57] evcleanup: replace _run_asap with `event_step' callback Eric Wong
2019-06-24  2:52 ` [PATCH 30/57] ds: remove pointless exit calls Eric Wong
2019-06-24  2:52 ` [PATCH 31/57] http|nntp: be explicit about bytes::length on rbuf Eric Wong
2019-06-24  2:52 ` [PATCH 32/57] ds: hoist out do_read from NNTP and HTTP Eric Wong
2019-06-24  2:52 ` [PATCH 33/57] nntp: simplify re-arming/requeue logic Eric Wong
2019-06-24  2:52 ` [PATCH 34/57] allow use of PerlIO layers for filesystem writes Eric Wong
2019-06-24  2:52 ` [PATCH 35/57] ds: deal better with FS-related errors IO buffers Eric Wong
2019-06-24  2:52 ` [PATCH 36/57] nntp: wait for writability before sending greeting Eric Wong
2019-06-24  2:52 ` [PATCH 37/57] nntp: NNTPS and NNTP+STARTTLS working Eric Wong
2019-06-24  2:52 ` [PATCH 38/57] certs/create-certs.perl: fix cert validity on 32-bit Eric Wong
2019-06-24  2:52 ` [PATCH 39/57] daemon: map inherited sockets to well-known schemes Eric Wong
2019-06-24  2:52 ` [PATCH 40/57] ds|nntp: use CORE::close on socket Eric Wong
2019-06-24  2:52 ` [PATCH 41/57] nntp: call SSL_shutdown in normal cases Eric Wong
2019-06-24  2:52 ` [PATCH 42/57] t/nntpd-tls: slow client connection test Eric Wong
2019-06-24  2:52 ` [PATCH 43/57] daemon: use SSL_MODE_RELEASE_BUFFERS Eric Wong
2019-06-24  2:52 ` [PATCH 44/57] ds: allow ->write callbacks to syswrite directly Eric Wong
2019-06-24  2:52 ` [PATCH 45/57] nntp: reduce allocations for greeting Eric Wong
2019-06-24  2:52 ` [PATCH 46/57] ds: always use EV_ADD with EV_SET Eric Wong
2019-06-24  2:52 ` [PATCH 47/57] nntp: simplify long response logic and fix nesting Eric Wong
2019-06-24  2:52 ` [PATCH 48/57] ds: flush_write runs ->write callbacks even if closed Eric Wong
2019-06-24  2:52 ` [PATCH 49/57] nntp: lazily allocate and stash rbuf Eric Wong
2019-06-24  2:52 ` [PATCH 50/57] ci: require IO::KQueue on FreeBSD, for now Eric Wong
2019-06-24  2:52 ` [PATCH 51/57] nntp: send greeting immediately for plain sockets Eric Wong
2019-06-24  2:52 ` [PATCH 52/57] daemon: set TCP_DEFER_ACCEPT on everything but NNTP Eric Wong
2019-06-24  2:52 ` [PATCH 53/57] daemon: use FreeBSD accept filters on non-NNTP Eric Wong
2019-06-24  2:52 ` [PATCH 54/57] ds: split out IO::KQueue-specific code Eric Wong
2019-06-24  5:24   ` Eric Wong
2019-06-24  2:52 ` [PATCH 55/57] ds: reimplement IO::Poll support to look like epoll Eric Wong
2019-06-24  2:52 ` [PATCH 56/57] Revert "ci: require IO::KQueue on FreeBSD, for now" Eric Wong
2019-06-24  2:52 ` [PATCH 57/57] ds: reduce overhead of tempfile creation Eric Wong
2019-06-24  5:25 ` [PATCH 58/57] Makefile: skip DSKQXS in global syntax check Eric Wong
2019-06-24 18:28 ` [PATCH 59/57] ds: ->write must not clobber empty wbuf array Eric Wong

Reply instructions:

You may reply publically 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

  List information: https://public-inbox.org/README

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

  git send-email \
    --in-reply-to=20190624025258.25592-8-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /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

user/dev discussion of public-inbox itself

Archives are clonable:
	git clone --mirror https://public-inbox.org/meta
	git clone --mirror http://czquwvybam4bgbro.onion/meta
	git clone --mirror http://hjrcffqmbrq6wope.onion/meta
	git clone --mirror http://ou63pmih66umazou.onion/meta

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta
	nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta
	nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta
	nntp://news.gmane.org/gmane.mail.public-inbox.general

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox