user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 0/4] Danga::Socket bundling cleanups
  2019-05-05  0:52  6% [PATCH 0/4] bundle Danga::Socket and Sys::Syscall Eric Wong
  2019-05-05  0:52  3% ` [PATCH 3/4] DS: remove unused fields and functions Eric Wong
@ 2019-05-08 19:18  7% ` Eric Wong
  1 sibling, 0 replies; 3+ results
From: Eric Wong @ 2019-05-08 19:18 UTC (permalink / raw)
  To: meta

Dropping some unused stuff, and a bugfix for an error path we never hit.
(all bugfixes are queued for the future maintainer via
 bug-Danga-Socket@rt.cpan.org )

Eric Wong (4):
  build: do not manify DS and Syscall pods
  syscall: drop readahead wrapper
  DS: drop unused "_undef" sub
  DS: epoll: fix misordered EPOLL_CTL_DEL call

 Makefile.PL                | 10 ++++++++++
 lib/PublicInbox/DS.pm      |  9 +--------
 lib/PublicInbox/Syscall.pm | 14 --------------
 3 files changed, 11 insertions(+), 22 deletions(-)

The "danga-bundle" is up to 10 patches, now; and dogfooded
on public-inbox.org for several days without problems.
Will merge to "master" soon:

      bundle Danga::Socket and Sys::Syscall
      listener: use EPOLLEXCLUSIVE for listen sockets
      DS: remove unused fields and functions
      DS: drop profiling support
      DS: workaround IO::Kqueue EINTR (mis-)handling
      DS: handle EINTR in IO::Poll path, too
      build: do not manify DS and Syscall pods
      syscall: drop readahead wrapper
      DS: drop unused "_undef" sub
      DS: epoll: fix misordered EPOLL_CTL_DEL call

^ permalink raw reply	[relevance 7%]

* [PATCH 0/4] bundle Danga::Socket and Sys::Syscall
@ 2019-05-05  0:52  6% Eric Wong
  2019-05-05  0:52  3% ` [PATCH 3/4] DS: remove unused fields and functions Eric Wong
  2019-05-08 19:18  7% ` [PATCH 0/4] Danga::Socket bundling cleanups Eric Wong
  0 siblings, 2 replies; 3+ results
From: Eric Wong @ 2019-05-05  0:52 UTC (permalink / raw)
  To: meta

This is probably our rarest and most esoteric dependencies
at the moment, so bundle them, add some features, and drop
unused ones.  It'll also give me an excuse to play with more
recent Linux kernel developments :>   More on this in [1/4]

Eric Wong (4):
  bundle Danga::Socket and Sys::Syscall
  listener: use EPOLLEXCLUSIVE for listen sockets
  DS: remove unused fields and functions
  DS: drop profiling support

 INSTALL                           |    4 -
 MANIFEST                          |    2 +
 TODO                              |    5 +-
 lib/PublicInbox/DS.pm             | 1051 +++++++++++++++++++++++++++++
 lib/PublicInbox/Daemon.pm         |    8 +-
 lib/PublicInbox/EvCleanup.pm      |   12 +-
 lib/PublicInbox/GitHTTPBackend.pm |    2 +-
 lib/PublicInbox/HTTP.pm           |   12 +-
 lib/PublicInbox/HTTPD/Async.pm    |    4 +-
 lib/PublicInbox/Listener.pm       |    4 +-
 lib/PublicInbox/NNTP.pm           |    6 +-
 lib/PublicInbox/ParentPipe.pm     |    2 +-
 lib/PublicInbox/Qspawn.pm         |    4 +-
 lib/PublicInbox/Syscall.pm        |  329 +++++++++
 t/git-http-backend.t              |    2 +-
 t/httpd-corner.t                  |    2 +-
 t/httpd-unix.t                    |    2 +-
 t/httpd.t                         |    2 +-
 t/nntp.t                          |    2 +-
 t/nntpd.t                         |    2 +-
 t/v2mirror.t                      |    2 +-
 t/v2writable.t                    |    4 +-
 22 files changed, 1419 insertions(+), 44 deletions(-)
 create mode 100644 lib/PublicInbox/DS.pm
 create mode 100644 lib/PublicInbox/Syscall.pm

-- 
EW


^ permalink raw reply	[relevance 6%]

* [PATCH 3/4] DS: remove unused fields and functions
  2019-05-05  0:52  6% [PATCH 0/4] bundle Danga::Socket and Sys::Syscall Eric Wong
@ 2019-05-05  0:52  3% ` Eric Wong
  2019-05-08 19:18  7% ` [PATCH 0/4] Danga::Socket bundling cleanups Eric Wong
  1 sibling, 0 replies; 3+ results
From: Eric Wong @ 2019-05-05  0:52 UTC (permalink / raw)
  To: meta

More will likely be dropped in the future, but drop the obvious
ones we aren't using, for now; especially since some of them are
set at ->new time and unavoidable.

This saves 579 bytes per-client on my 64-bit Debian stable
system as measured by Devel::Size::total_size from
PublicInbox::HTTP::event_read.  This adds up in C10K or C100K
situations.

Things we drop are:

* corked - MSG_MORE requires fewer syscalls
* read_push_back - tried to use it, ate CPU with slow clients
* IP/port fields - accept() already returns what we care about
---
 lib/PublicInbox/DS.pm | 199 ------------------------------------------
 1 file changed, 199 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 3ccc275..f181eee 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -29,15 +29,8 @@ use fields ('sock',              # underlying socket
             '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
             'write_set_watch',   # bool: true if we internally set watch_write rather than by a subclass
-            'read_push_back',    # arrayref of "pushed-back" read data the application didn't want
             'closed',            # bool: socket is closed
-            'corked',            # bool: socket is corked
             'event_watch',       # bitmask of events the client is interested in (POLLIN,OUT,etc.)
-            'peer_v6',           # bool: cached; if peer is an IPv6 address
-            'peer_ip',           # cached stringified IP address of $sock
-            'peer_port',         # cached port number of $sock
-            'local_ip',          # cached stringified IP address of local end of $sock
-            'local_port',        # cached port number of local end of $sock
             'writer_func',       # subref which does writing.  must return bytes written (or undef) and set $! on errors
             );
 
@@ -46,7 +39,6 @@ use Errno  qw(EINPROGRESS EWOULDBLOCK EISCONN ENOTSOCK
 use Socket qw(IPPROTO_TCP);
 use Carp   qw(croak confess);
 
-use constant TCP_CORK => ($^O eq "linux" ? 3 : 0); # FIXME: not hard-coded (Linux-specific too)
 use constant DebugLevel => 0;
 
 use constant POLLIN        => 1;
@@ -61,7 +53,6 @@ our (
      $HaveEpoll,                 # Flag -- is epoll available?  initially undefined.
      $HaveKQueue,
      %DescriptorMap,             # fd (num) -> PublicInbox::DS object
-     %PushBackSet,               # fd (num) -> PublicInbox::DS (fds with pushed back read data)
      $Epoll,                     # Global epoll fd (for epoll mode only)
      $KQueue,                    # Global kqueue fd (for kqueue mode only)
      @ToClose,                   # sockets to close when event loop is done
@@ -93,7 +84,6 @@ Reset all state
 =cut
 sub Reset {
     %DescriptorMap = ();
-    %PushBackSet = ();
     @ToClose = ();
     %OtherFds = ();
     $LoopTimeout = -1;  # no timeout by default
@@ -598,27 +588,6 @@ sub SetPostLoopCallback {
 # for pushed-back data, and close pending connections.  returns 1
 # if event loop should continue, or 0 to shut it all down.
 sub PostEventLoop {
-    # fire read events for objects with pushed-back read data
-    my $loop = 1;
-    while ($loop) {
-        $loop = 0;
-        foreach my $fd (keys %PushBackSet) {
-            my PublicInbox::DS $pob = $PushBackSet{$fd};
-
-            # a previous event_read invocation could've closed a
-            # connection that we already evaluated in "keys
-            # %PushBackSet", so skip ones that seem to have
-            # disappeared.  this is expected.
-            next unless $pob;
-
-            die "ASSERT: the $pob socket has no read_push_back" unless @{$pob->{read_push_back}};
-            next unless (! $pob->{closed} &&
-                         $pob->{event_watch} & POLLIN);
-            $loop = 1;
-            $pob->event_read;
-        }
-    }
-
     # now we can close sockets that wanted to close during our event processing.
     # (we didn't want to close them during the loop, as we didn't want fd numbers
     #  being reused and confused during the event loop)
@@ -682,8 +651,6 @@ sub new {
     $self->{write_buf_offset} = 0;
     $self->{write_buf_size} = 0;
     $self->{closed} = 0;
-    $self->{corked} = 0;
-    $self->{read_push_back} = [];
 
     my $ev = $self->{event_watch} = POLLERR|POLLHUP|POLLNVAL;
 
@@ -723,49 +690,6 @@ retry:
 ### I N S T A N C E   M E T H O D S
 #####################################################################
 
-=head2 C<< $obj->tcp_cork( $boolean ) >>
-
-Turn TCP_CORK on or off depending on the value of I<boolean>.
-
-=cut
-sub tcp_cork {
-    my PublicInbox::DS $self = $_[0];
-    my $val = $_[1];
-
-    # make sure we have a socket
-    return unless $self->{sock};
-    return if $val == $self->{corked};
-
-    my $rv;
-    if (TCP_CORK) {
-        $rv = setsockopt($self->{sock}, IPPROTO_TCP, TCP_CORK,
-                         pack("l", $val ? 1 : 0));
-    } else {
-        # FIXME: implement freebsd *PUSH sockopts
-        $rv = 1;
-    }
-
-    # if we failed, close (if we're not already) and warn about the error
-    if ($rv) {
-        $self->{corked} = $val;
-    } else {
-        if ($! == EBADF || $! == ENOTSOCK) {
-            # internal state is probably corrupted; warn and then close if
-            # we're not closed already
-            warn "setsockopt: $!";
-            $self->close('tcp_cork_failed');
-        } elsif ($! == ENOPROTOOPT || $!{ENOTSOCK} || $!{EOPNOTSUPP}) {
-            # TCP implementation doesn't support corking, so just ignore it
-            # or we're trying to tcp-cork a non-socket (like a socketpair pipe
-            # which is acting like a socket, which Perlbal does for child
-            # processes acting like inetd-like web servers)
-        } else {
-            # some other error; we should never hit here, but if we do, die
-            die "setsockopt: $!";
-        }
-    }
-}
-
 =head2 C<< $obj->steal_socket() >>
 
 Basically returns our socket and makes it so that we don't try to close it,
@@ -828,10 +752,6 @@ sub _cleanup {
     # preventing the object from being destroyed
     $self->{write_buf} = [];
 
-    # uncork so any final data gets sent.  only matters if the person closing
-    # us forgot to do it, but we do it to be safe.
-    $self->tcp_cork(0);
-
     # if we're using epoll, we have to remove this from our epoll fd so we stop getting
     # notifications about it
     if ($HaveEpoll && $self->{fd}) {
@@ -843,7 +763,6 @@ sub _cleanup {
 
     # now delete from mappings.  this fd no longer belongs to us, so we don't want
     # to get alerts for it if it becomes writable/readable/etc.
-    delete $PushBackSet{$self->{fd}};
     delete $PLCMap{$self->{fd}};
 
     # we explicitly don't delete from DescriptorMap here until we
@@ -1020,19 +939,6 @@ sub on_incomplete_write {
     $self->watch_write(1);
 }
 
-=head2 C<< $obj->push_back_read( $buf ) >>
-
-Push back I<buf> (a scalar or scalarref) into the read stream. Useful if you read
-more than you need to and want to return this data on the next "read".
-
-=cut
-sub push_back_read {
-    my PublicInbox::DS $self = shift;
-    my $buf = shift;
-    push @{$self->{read_push_back}}, ref $buf ? $buf : \$buf;
-    $PushBackSet{$self->{fd}} = $self;
-}
-
 =head2 C<< $obj->read( $bytecount ) >>
 
 Read at most I<bytecount> bytes from the underlying handle; returns scalar
@@ -1046,22 +952,6 @@ sub read {
     my $buf;
     my $sock = $self->{sock};
 
-    if (@{$self->{read_push_back}}) {
-        $buf = shift @{$self->{read_push_back}};
-        my $len = length($$buf);
-
-        if ($len <= $bytes) {
-            delete $PushBackSet{$self->{fd}} unless @{$self->{read_push_back}};
-            return $buf;
-        } else {
-            # if the pushed back read is too big, we have to split it
-            my $overflow = substr($$buf, $bytes);
-            $buf = substr($$buf, 0, $bytes);
-            unshift @{$self->{read_push_back}}, \$overflow;
-            return \$buf;
-        }
-    }
-
     # if this is too high, perl quits(!!).  reports on mailing lists
     # don't seem to point to a universal answer.  5MB worked for some,
     # crashed for others.  1MB works for more people.  let's go with 1MB
@@ -1216,91 +1106,6 @@ sub debugmsg {
     printf STDERR ">>> $fmt\n", @args;
 }
 
-
-=head2 C<< $obj->peer_ip_string() >>
-
-Returns the string describing the peer's IP
-
-=cut
-sub peer_ip_string {
-    my PublicInbox::DS $self = shift;
-    return _undef("peer_ip_string undef: no sock") unless $self->{sock};
-    return $self->{peer_ip} if defined $self->{peer_ip};
-
-    my $pn = getpeername($self->{sock});
-    return _undef("peer_ip_string undef: getpeername") unless $pn;
-
-    my ($port, $iaddr) = eval {
-        if (length($pn) >= 28) {
-            return Socket6::unpack_sockaddr_in6($pn);
-        } else {
-            return Socket::sockaddr_in($pn);
-        }
-    };
-
-    if ($@) {
-        $self->{peer_port} = "[Unknown peerport '$@']";
-        return "[Unknown peername '$@']";
-    }
-
-    $self->{peer_port} = $port;
-
-    if (length($iaddr) == 4) {
-        return $self->{peer_ip} = Socket::inet_ntoa($iaddr);
-    } else {
-        $self->{peer_v6} = 1;
-        return $self->{peer_ip} = Socket6::inet_ntop(Socket6::AF_INET6(),
-                                                     $iaddr);
-    }
-}
-
-=head2 C<< $obj->peer_addr_string() >>
-
-Returns the string describing the peer for the socket which underlies this
-object in form "ip:port"
-
-=cut
-sub peer_addr_string {
-    my PublicInbox::DS $self = shift;
-    my $ip = $self->peer_ip_string
-        or return undef;
-    return $self->{peer_v6} ?
-        "[$ip]:$self->{peer_port}" :
-        "$ip:$self->{peer_port}";
-}
-
-=head2 C<< $obj->local_ip_string() >>
-
-Returns the string describing the local IP
-
-=cut
-sub local_ip_string {
-    my PublicInbox::DS $self = shift;
-    return _undef("local_ip_string undef: no sock") unless $self->{sock};
-    return $self->{local_ip} if defined $self->{local_ip};
-
-    my $pn = getsockname($self->{sock});
-    return _undef("local_ip_string undef: getsockname") unless $pn;
-
-    my ($port, $iaddr) = Socket::sockaddr_in($pn);
-    $self->{local_port} = $port;
-
-    return $self->{local_ip} = Socket::inet_ntoa($iaddr);
-}
-
-=head2 C<< $obj->local_addr_string() >>
-
-Returns the string describing the local end of the socket which underlies this
-object in form "ip:port"
-
-=cut
-sub local_addr_string {
-    my PublicInbox::DS $self = shift;
-    my $ip = $self->local_ip_string;
-    return $ip ? "$ip:$self->{local_port}" : undef;
-}
-
-
 =head2 C<< $obj->as_string() >>
 
 Returns a string describing this socket.
@@ -1311,10 +1116,6 @@ sub as_string {
     my $rw = "(" . ($self->{event_watch} & POLLIN ? 'R' : '') .
                    ($self->{event_watch} & POLLOUT ? 'W' : '') . ")";
     my $ret = ref($self) . "$rw: " . ($self->{closed} ? "closed" : "open");
-    my $peer = $self->peer_addr_string;
-    if ($peer) {
-        $ret .= " to " . $self->peer_addr_string;
-    }
     return $ret;
 }
 
-- 
EW


^ permalink raw reply related	[relevance 3%]

Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2019-05-05  0:52  6% [PATCH 0/4] bundle Danga::Socket and Sys::Syscall Eric Wong
2019-05-05  0:52  3% ` [PATCH 3/4] DS: remove unused fields and functions Eric Wong
2019-05-08 19:18  7% ` [PATCH 0/4] Danga::Socket bundling cleanups Eric Wong

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).