From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 1/9] ds: simplify write buffer accounting
Date: Mon, 10 Jun 2019 05:18:38 +0000 [thread overview]
Message-ID: <20190610051846.26757-2-e@80x24.org> (raw)
In-Reply-To: <20190610051846.26757-1-e@80x24.org>
Keeping track of write_buf_size was redundant and pointless when
we can simply check the number of elements in the buffer array.
Multiple sources of truth leads to confusion; confusion leads to
bugs.
Finally, rename the prefixes to 'wbuf' to ensure we loudly
(instead of silently) break any external dependencies being
ported over from Danga::Socket, as further changes are pending.
---
lib/PublicInbox/DS.pm | 41 ++++++++++++++--------------------
lib/PublicInbox/HTTP.pm | 6 ++---
lib/PublicInbox/HTTPD/Async.pm | 2 +-
lib/PublicInbox/NNTP.pm | 8 +++----
4 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 03612ce8..172a9f52 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -27,9 +27,8 @@ use PublicInbox::Syscall qw(:epoll);
use fields ('sock', # underlying socket
'fd', # numeric file descriptor
- 'write_buf', # arrayref of scalars, scalarrefs, or coderefs to write
- 'write_buf_offset', # offset into first array of write_buf to start writing at
- 'write_buf_size', # total length of data in all write_buf items
+ 'wbuf', # arrayref of scalars, scalarrefs, or coderefs to write
+ 'wbuf_off', # offset into first element of wbuf to start writing at
'closed', # bool: socket is closed
'event_watch', # bitmask of events the client is interested in (POLLIN,OUT,etc.)
);
@@ -449,9 +448,8 @@ sub new {
unless $sock && $fd;
$self->{fd} = $fd;
- $self->{write_buf} = [];
- $self->{write_buf_offset} = 0;
- $self->{write_buf_size} = 0;
+ $self->{wbuf} = [];
+ $self->{wbuf_off} = 0;
$self->{closed} = 0;
my $ev = $self->{event_watch} = POLLERR|POLLHUP|POLLNVAL;
@@ -552,7 +550,7 @@ sub _cleanup {
# we need to flush our write buffer, as there may
# be self-referential closures (sub { $client->close })
# preventing the object from being destroyed
- $self->{write_buf} = [];
+ @{$self->{wbuf}} = ();
# if we're using epoll, we have to remove this from our epoll fd so we stop getting
# notifications about it
@@ -612,12 +610,12 @@ sub write {
# just queue data if there's already a wait
my $need_queue;
+ my $wbuf = $self->{wbuf};
if (defined $data) {
$bref = ref $data ? $data : \$data;
- if ($self->{write_buf_size}) {
- push @{$self->{write_buf}}, $bref;
- $self->{write_buf_size} += ref $bref eq "SCALAR" ? length($$bref) : 1;
+ if (scalar @$wbuf) {
+ push @$wbuf, $bref;
return 0;
}
@@ -629,7 +627,7 @@ sub write {
WRITE:
while (1) {
- return 1 unless $bref ||= $self->{write_buf}[0];
+ return 1 unless $bref ||= $wbuf->[0];
my $len;
eval {
@@ -638,8 +636,7 @@ sub write {
if ($@) {
if (UNIVERSAL::isa($bref, "CODE")) {
unless ($need_queue) {
- $self->{write_buf_size}--; # code refs are worth 1
- shift @{$self->{write_buf}};
+ shift @$wbuf;
}
$bref->();
@@ -654,9 +651,9 @@ sub write {
die "Write error: $@ <$bref>";
}
- my $to_write = $len - $self->{write_buf_offset};
+ my $to_write = $len - $self->{wbuf_off};
my $written = syswrite($self->{sock}, $$bref, $to_write,
- $self->{write_buf_offset});
+ $self->{wbuf_off});
if (! defined $written) {
if ($! == EPIPE) {
@@ -665,8 +662,7 @@ sub write {
# since connection has stuff to write, it should now be
# interested in pending writes:
if ($need_queue) {
- push @{$self->{write_buf}}, $bref;
- $self->{write_buf_size} += $len;
+ push @$wbuf, $bref;
}
$self->watch_write(1);
return 0;
@@ -681,19 +677,17 @@ sub write {
DebugLevel >= 2 && $self->debugmsg("Wrote PARTIAL %d bytes to %d",
$written, $self->{fd});
if ($need_queue) {
- push @{$self->{write_buf}}, $bref;
- $self->{write_buf_size} += $len;
+ push @$wbuf, $bref;
}
# since connection has stuff to write, it should now be
# interested in pending writes:
- $self->{write_buf_offset} += $written;
- $self->{write_buf_size} -= $written;
+ $self->{wbuf_off} += $written;
$self->on_incomplete_write;
return 0;
} elsif ($written == $to_write) {
DebugLevel >= 2 && $self->debugmsg("Wrote ALL %d bytes to %d (nq=%d)",
$written, $self->{fd}, $need_queue);
- $self->{write_buf_offset} = 0;
+ $self->{wbuf_off} = 0;
$self->watch_write(0);
# this was our only write, so we can return immediately
@@ -702,8 +696,7 @@ sub write {
# can't be anything else to write.
return 1 if $need_queue;
- $self->{write_buf_size} -= $written;
- shift @{$self->{write_buf}};
+ shift @$wbuf;
undef $bref;
next WRITE;
}
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 977614b4..fd103251 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -260,7 +260,7 @@ sub getline_cb ($$$) {
$write->($buf); # may close in PublicInbox::DS::write
unless ($self->{closed}) {
my $next = $self->{pull};
- if ($self->{write_buf_size}) {
+ if (scalar @{$self->{wbuf}}) {
$self->write($next);
} else {
PublicInbox::EvCleanup::asap($next);
@@ -315,7 +315,7 @@ use constant MSG_MORE => ($^O eq 'linux') ? 0x8000 : 0;
sub more ($$) {
my $self = $_[0];
return if $self->{closed};
- if (MSG_MORE && !$self->{write_buf_size}) {
+ if (MSG_MORE && !scalar(@{$self->{wbuf}})) {
my $n = send($self->{sock}, $_[1], MSG_MORE);
if (defined $n) {
my $nlen = length($_[1]) - $n;
@@ -487,7 +487,7 @@ sub close {
# for graceful shutdown in PublicInbox::Daemon:
sub busy () {
my ($self) = @_;
- ($self->{rbuf} ne '' || $self->{env} || $self->{write_buf_size});
+ ($self->{rbuf} ne '' || $self->{env} || scalar(@{$self->{wbuf}}));
}
1;
diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index dbe8a84a..60701085 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -46,7 +46,7 @@ sub main_cb ($$$) {
if ($r) {
$fh->write($$bref);
unless ($http->{closed}) { # PublicInbox::DS sets this
- if ($http->{write_buf_size}) {
+ if (scalar @{$http->{wbuf}}) {
$self->watch_read(0);
$http->write(restart_read_cb($self));
}
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index be80560f..b62c2187 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -619,7 +619,7 @@ sub long_response ($$) {
update_idle_time($self);
check_read($self);
}
- } elsif ($more) { # $self->{write_buf_size}:
+ } elsif ($more) { # scalar @{$self->{wbuf}}:
# no recursion, schedule another call ASAP
# but only after all pending writes are done
update_idle_time($self);
@@ -925,7 +925,7 @@ use constant MSG_MORE => ($^O eq 'linux') ? 0x8000 : 0;
sub do_more ($$) {
my ($self, $data) = @_;
- if (MSG_MORE && !$self->{write_buf_size}) {
+ if (MSG_MORE && !scalar(@{$self->{wbuf}})) {
my $n = send($self->{sock}, $data, MSG_MORE);
if (defined $n) {
my $dlen = length($data);
@@ -1004,8 +1004,8 @@ sub not_idle_long ($$) {
# for graceful shutdown in PublicInbox::Daemon:
sub busy {
my ($self, $now) = @_;
- ($self->{rbuf} ne '' || $self->{long_res} || $self->{write_buf_size} ||
- not_idle_long($self, $now));
+ ($self->{rbuf} ne '' || $self->{long_res} ||
+ scalar(@{$self->{wbuf}}) || not_idle_long($self, $now));
}
1;
--
EW
next prev parent reply other threads:[~2019-06-10 5:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-10 5:18 [PATCH 0/9] ds: Diet Socket Eric Wong
2019-06-10 5:18 ` Eric Wong [this message]
2019-06-10 5:18 ` [PATCH 2/9] ds: cleanup Errno imports and favor constant comparisons Eric Wong
2019-06-10 5:18 ` [PATCH 3/9] ds: reduce Errno imports and drop ->close reason Eric Wong
2019-06-10 5:18 ` [PATCH 4/9] ds: remove {fd} field Eric Wong
2019-06-10 5:18 ` [PATCH 5/9] ds: remove steal_socket method Eric Wong
2019-06-10 5:18 ` [PATCH 6/9] nntp: use sysread to append to existing buffer Eric Wong
2019-06-10 5:18 ` [PATCH 7/9] ds: remove read method, here, too Eric Wong
2019-06-10 5:18 ` [PATCH 8/9] ds: do not distinguish between POLLHUP and POLLERR Eric Wong
2019-06-10 5:18 ` [PATCH 9/9] ds: stop caring about event flags set by epoll/poll/kqueue Eric Wong
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
List information: http://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=20190610051846.26757-2-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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
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).