]> Sergey Matveev's repositories - public-inbox.git/commitdiff
ds: remove fields.pm usage
authorEric Wong <e@yhbt.net>
Sat, 27 Jun 2020 10:03:38 +0000 (10:03 +0000)
committerEric Wong <e@yhbt.net>
Sun, 28 Jun 2020 22:27:15 +0000 (22:27 +0000)
Since the removal of pseudo-hash support in Perl 5.10, the
"fields" module no longer provides the space or speed benefits
it did in 5.8.  It also does not allow for compile-time checks,
only run-time checks.

To me, the extra developer overhead in maintaining "use fields"
args has become a hassle.  None of our non-DS-related code uses
fields.pm, nor do any of our current dependencies.  In fact,
Danga::Socket (which DS was originally forked from) and its
subclasses are the only fields.pm users I've ever encountered in
the wild.  Removing fields may make our code more approachable
to other Perl hackers.

So stop using fields.pm and locked hashes, but continue to
document what fields do for non-trivial classes.

13 files changed:
lib/PublicInbox/DS.pm
lib/PublicInbox/DirIdle.pm
lib/PublicInbox/GitAsyncCat.pm
lib/PublicInbox/HTTP.pm
lib/PublicInbox/HTTPD/Async.pm
lib/PublicInbox/IMAP.pm
lib/PublicInbox/InboxIdle.pm
lib/PublicInbox/Listener.pm
lib/PublicInbox/NNTP.pm
lib/PublicInbox/NNTPdeflate.pm
lib/PublicInbox/ParentPipe.pm
lib/PublicInbox/Sigfd.pm
xt/mem-imapd-tls.t

index aa65b2d364259d97003f3c4b83b79b301c4003f7..da68802dda94a82da154e156207442b4641bdd99 100644 (file)
 # Bugs encountered were reported to bug-Danga-Socket@rt.cpan.org,
 # fixed in Danga::Socket 1.62 and visible at:
 # https://rt.cpan.org/Public/Dist/Display.html?Name=Danga-Socket
+#
+# fields:
+# sock: underlying socket
+# rbuf: scalarref, usually undef
+# wbuf: arrayref of coderefs or tmpio (autovivified))
+#        (tmpio = [ GLOB, offset, [ length ] ])
 package PublicInbox::DS;
 use strict;
 use bytes;
@@ -22,19 +28,10 @@ use Fcntl qw(SEEK_SET :DEFAULT O_APPEND);
 use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
 use parent qw(Exporter);
 our @EXPORT_OK = qw(now msg_more);
-use warnings;
 use 5.010_001;
 use Scalar::Util qw(blessed);
-
 use PublicInbox::Syscall qw(:epoll);
 use PublicInbox::Tmpfile;
-
-use fields ('sock',              # underlying socket
-            'rbuf',              # scalarref, usually undef
-            'wbuf', # arrayref of coderefs or tmpio (autovivified))
-                    # (tmpio = [ GLOB, offset, [ length ] ])
-            );
-
 use Errno qw(EAGAIN EINVAL);
 use Carp qw(confess carp);
 
@@ -328,8 +325,6 @@ This is normally (always?) called from your subclass via:
 =cut
 sub new {
     my ($self, $sock, $ev) = @_;
-    $self = fields::new($self) unless ref $self;
-
     $self->{sock} = $sock;
     my $fd = fileno($sock);
 
index ffceda66530b73d9a75abf162b60faaea565f0a5..fbbc9531a20a4971c23090ed306f4d34681bd2bb 100644 (file)
@@ -4,8 +4,7 @@
 # Used by public-inbox-watch for Maildir (and possibly MH in the future)
 package PublicInbox::DirIdle;
 use strict;
-use base 'PublicInbox::DS';
-use fields qw(inot);
+use parent 'PublicInbox::DS';
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 use PublicInbox::In2Tie;
 
@@ -24,7 +23,7 @@ if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) {
 
 sub new {
        my ($class, $dirs, $cb) = @_;
-       my $self = fields::new($class);
+       my $self = bless {}, $class;
        my $inot;
        if ($ino_cls) {
                $inot = $ino_cls->new or die "E: $ino_cls->new: $!";
index 098101aed00843633eb239443a1a9c61f3cdf937..0b777204a7ccfe021fe102c3218c2af4de5565ce 100644 (file)
 package PublicInbox::GitAsyncCat;
 use strict;
 use parent qw(PublicInbox::DS Exporter);
-use fields qw(git);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 our @EXPORT = qw(git_async_cat);
 
 sub _add {
        my ($class, $git) = @_;
-       my $self = fields::new($class);
        $git->batch_prepare;
+       my $self = bless { git => $git }, $class;
        $self->SUPER::new($git->{in}, EPOLLIN|EPOLLET);
-       $self->{git} = $git;
        \undef; # this is a true ref()
 }
 
index 6ccf20592405e2ee3732d79283b8ac262c26cd6f..8281746538e2df420a7003a8fc62cdd2dab159a9 100644 (file)
@@ -6,12 +6,21 @@
 # to learn different ways to admin both NNTP and HTTP components.
 # There's nothing which depends on public-inbox, here.
 # Each instance of this class represents a HTTP client socket
-
+#
+# fields:
+# httpd: PublicInbox::HTTPD ref
+# env: PSGI env hashref
+# input_left: bytes left to read in request body (e.g. POST/PUT)
+# remote_addr: remote IP address as a string (e.g. "127.0.0.1")
+# remote_port: peer port
+# forward: response body object, response to ->getline + ->close
+# alive: HTTP keepalive state:
+#      0: drop connection when done
+#      1: keep connection when done
+#      2: keep connection, chunk responses
 package PublicInbox::HTTP;
 use strict;
-use warnings;
-use base qw(PublicInbox::DS);
-use fields qw(httpd env input_left remote_addr remote_port forward alive);
+use parent qw(PublicInbox::DS);
 use bytes (); # only for bytes::length
 use Fcntl qw(:seek);
 use Plack::HTTPParser qw(parse_http_request); # XS or pure Perl
@@ -56,7 +65,7 @@ sub http_date () {
 
 sub new ($$$) {
        my ($class, $sock, $addr, $httpd) = @_;
-       my $self = fields::new($class);
+       my $self = bless { httpd => $httpd }, $class;
        my $ev = EPOLLIN;
        my $wbuf;
        if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
@@ -64,12 +73,10 @@ sub new ($$$) {
                $ev = PublicInbox::TLS::epollbit();
                $wbuf = [ \&PublicInbox::DS::accept_tls_step ];
        }
-       $self->SUPER::new($sock, $ev | EPOLLONESHOT);
-       $self->{httpd} = $httpd;
        $self->{wbuf} = $wbuf if $wbuf;
        ($self->{remote_addr}, $self->{remote_port}) =
                PublicInbox::Daemon::host_with_port($addr);
-       $self;
+       $self->SUPER::new($sock, $ev | EPOLLONESHOT);
 }
 
 sub event_step { # called by PublicInbox::DS
index 35075d344b0c2848fb8b12efd014c01093b8aa54..87a6a5f9cf015dfab9215231d3d34dc7afdcdb58 100644 (file)
@@ -6,11 +6,16 @@
 # The name of this key is not even stable!
 # Currently intended for use with read-only pipes with expensive
 # processes such as git-http-backend(1), cgit(1)
+#
+# fields:
+# http: PublicInbox::HTTP ref
+# fh: PublicInbox::HTTP::{Identity,Chunked} ref (can ->write + ->close)
+# cb: initial read callback
+# arg: arg for {cb}
+# end_obj: CODE or object which responds to ->event_step when ->close is called
 package PublicInbox::HTTPD::Async;
 use strict;
-use warnings;
-use base qw(PublicInbox::DS);
-use fields qw(http fh cb arg end_obj);
+use parent qw(PublicInbox::DS);
 use Errno qw(EAGAIN);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 
@@ -27,14 +32,13 @@ sub new {
                die '$end_obj unsupported w/o $io' if $end_obj;
                return;
        }
-
-       my $self = fields::new($class);
+       my $self = bless {
+               cb => $cb, # initial read callback
+               arg => $arg, # arg for $cb
+               end_obj => $end_obj, # like END{}, can ->event_step
+       }, $class;
        IO::Handle::blocking($io, 0);
        $self->SUPER::new($io, EPOLLIN | EPOLLET);
-       $self->{cb} = $cb; # initial read callback
-       $self->{arg} = $arg; # arg for $cb
-       $self->{end_obj} = $end_obj; # like END{}, can ->event_step
-       $self;
 }
 
 sub event_step {
index 9ad74de837fd59045c82471ad587a168901bd004..e0602143835baed35dcc164b63da0d514acb3f55 100644 (file)
 #   as a 50K uint16_t array (via pack("S*", ...)).  "UID offset"
 #   is the offset from {uid_base} which determines the start of
 #   the mailbox slice.
-
+#
+# fields:
+# imapd: PublicInbox::IMAPD ref
+# ibx: PublicInbox::Inbox ref
+# long_cb: long_response private data
+# uid_base: base UID for mailbox slice (0-based)
+# -login_tag: IMAP TAG for LOGIN
+# -idle_tag: IMAP response tag for IDLE
+# uo2m: UID-to-MSN mapping
 package PublicInbox::IMAP;
 use strict;
-use base qw(PublicInbox::DS);
-use fields qw(imapd ibx long_cb -login_tag
-       uid_base -idle_tag uo2m);
+use parent qw(PublicInbox::DS);
 use PublicInbox::Eml;
 use PublicInbox::EmlContentFoo qw(parse_content_disposition);
 use PublicInbox::DS qw(now);
@@ -34,7 +40,6 @@ use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
 use PublicInbox::GitAsyncCat;
 use Text::ParseWords qw(parse_line);
 use Errno qw(EAGAIN);
-use Hash::Util qw(unlock_hash); # dependency of fields for perl 5.10+, anyways
 use PublicInbox::Search;
 use PublicInbox::IMAPsearchqp;
 *mdocid = \&PublicInbox::Search::mdocid;
@@ -107,8 +112,7 @@ sub greet ($) {
 
 sub new ($$$) {
        my ($class, $sock, $imapd) = @_;
-       my $self = fields::new('PublicInbox::IMAP_preauth');
-       unlock_hash(%$self);
+       my $self = bless { imapd => $imapd }, 'PublicInbox::IMAP_preauth';
        my $ev = EPOLLIN;
        my $wbuf;
        if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
@@ -117,7 +121,6 @@ sub new ($$$) {
                $wbuf = [ \&PublicInbox::DS::accept_tls_step, \&greet ];
        }
        $self->SUPER::new($sock, $ev | EPOLLONESHOT);
-       $self->{imapd} = $imapd;
        if ($wbuf) {
                $self->{wbuf} = $wbuf;
        } else {
index ba8200aef05b366220d89e533d9f13c68d48a26f..59cb833fd5a10b466ae74ac8a65d91df3987ffa6 100644 (file)
@@ -1,10 +1,13 @@
 # Copyright (C) 2020 all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
+# fields:
+# pi_config: PublicInbox::Config ref
+# inot: Linux::Inotify2-like object
+# pathmap => { inboxdir => [ ibx, watch1, watch2, watch3... ] } mapping
 package PublicInbox::InboxIdle;
 use strict;
-use base qw(PublicInbox::DS);
-use fields qw(pi_config inot pathmap);
+use parent qw(PublicInbox::DS);
 use Cwd qw(abs_path);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 my $IN_MODIFY = 0x02; # match Linux inotify
@@ -50,7 +53,7 @@ sub refresh {
 
 sub new {
        my ($class, $pi_config) = @_;
-       my $self = fields::new($class);
+       my $self = bless {}, $class;
        my $inot;
        if ($ino_cls) {
                $inot = $ino_cls->new or die "E: $ino_cls->new: $!";
index eb7dd8d46cc684fefb5dfa6c221557173c8a0ad2..2e0fc248fe7d49067c36e71f5226b300c28e3c15 100644 (file)
@@ -4,10 +4,8 @@
 # Used by -nntpd for listen sockets
 package PublicInbox::Listener;
 use strict;
-use warnings;
-use base 'PublicInbox::DS';
+use parent 'PublicInbox::DS';
 use Socket qw(SOL_SOCKET SO_KEEPALIVE IPPROTO_TCP TCP_NODELAY);
-use fields qw(post_accept);
 use IO::Handle;
 use PublicInbox::Syscall qw(EPOLLIN EPOLLEXCLUSIVE EPOLLET);
 use Errno qw(EAGAIN ECONNABORTED EPERM);
@@ -23,10 +21,8 @@ sub new ($$$) {
        setsockopt($s, SOL_SOCKET, SO_KEEPALIVE, 1);
        setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1); # ignore errors on non-TCP
        listen($s, 1024);
-       my $self = fields::new($class);
+       my $self = bless { post_accept => $cb }, $class;
        $self->SUPER::new($s, EPOLLIN|EPOLLET|EPOLLEXCLUSIVE);
-       $self->{post_accept} = $cb;
-       $self
 }
 
 sub event_step {
index 76f14bbd97d9bb943ad50bdddee4635c8de691f3..9d91544abd309d7186ceae91e338129a42f1e361 100644 (file)
@@ -2,11 +2,14 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 #
 # Each instance of this represents a NNTP client socket
+# fields:
+# nntpd: PublicInbox::NNTPD ref
+# article: per-session current article number
+# ng: PublicInbox::Inbox ref
+# long_cb: long_response private data
 package PublicInbox::NNTP;
 use strict;
-use warnings;
-use base qw(PublicInbox::DS);
-use fields qw(nntpd article ng long_cb);
+use parent qw(PublicInbox::DS);
 use PublicInbox::MID qw(mid_escape $MID_EXTRACT);
 use PublicInbox::Eml;
 use POSIX qw(strftime);
@@ -45,7 +48,7 @@ sub greet ($) { $_[0]->write($_[0]->{nntpd}->{greet}) };
 
 sub new ($$$) {
        my ($class, $sock, $nntpd) = @_;
-       my $self = fields::new($class);
+       my $self = bless { nntpd => $nntpd }, $class;
        my $ev = EPOLLIN;
        my $wbuf;
        if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
@@ -54,7 +57,6 @@ sub new ($$$) {
                $wbuf = [ \&PublicInbox::DS::accept_tls_step, \&greet ];
        }
        $self->SUPER::new($sock, $ev | EPOLLONESHOT);
-       $self->{nntpd} = $nntpd;
        if ($wbuf) {
                $self->{wbuf} = $wbuf;
        } else {
index dec88aba3a5f22d90af5c3fd8afb6a9504362f4c..02af935f0dfc3be4dcea4f884b5eb83bf559b198 100644 (file)
 #       efficient in terms of server memory usage.
 package PublicInbox::NNTPdeflate;
 use strict;
-use warnings;
 use 5.010_001;
-use base qw(PublicInbox::NNTP);
+use parent qw(PublicInbox::NNTP);
 use Compress::Raw::Zlib;
-use Hash::Util qw(unlock_hash); # dependency of fields for perl 5.10+, anyways
 
 my %IN_OPT = (
        -Bufsize => PublicInbox::NNTP::LINE_MAX,
@@ -53,7 +51,6 @@ sub enable {
                $self->res('403 Unable to activate compression');
                return;
        }
-       unlock_hash(%$self);
        $self->res('206 Compression active');
        bless $self, $class;
        $self->{zin} = $in;
index f62f011bbe3abf53b98a3be33c6b64d8033a3183..538b5632c623851aebaed6662238ac3406a9b220 100644 (file)
@@ -5,17 +5,13 @@
 # notified if the master process dies.
 package PublicInbox::ParentPipe;
 use strict;
-use warnings;
-use base qw(PublicInbox::DS);
-use fields qw(cb);
+use parent qw(PublicInbox::DS);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
 
 sub new ($$$) {
        my ($class, $pipe, $worker_quit) = @_;
-       my $self = fields::new($class);
+       my $self = bless { cb => $worker_quit }, $class;
        $self->SUPER::new($pipe, EPOLLIN|EPOLLONESHOT);
-       $self->{cb} = $worker_quit;
-       $self;
 }
 
 # master process died, time to call worker_quit ourselves
index 17456592a7e52718d9ca595b282962f53c898ea3..bf91bb3774f1b71939ef18283ca344834bfb61e5 100644 (file)
@@ -1,9 +1,11 @@
 # Copyright (C) 2019-2020 all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# Wraps a signalfd (or similar) for PublicInbox::DS
+# fields: (sig: hashref similar to %SIG, but signal numbers as keys)
 package PublicInbox::Sigfd;
 use strict;
 use parent qw(PublicInbox::DS);
-use fields qw(sig); # hashref similar to %SIG, but signal numbers as keys
 use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET SFD_NONBLOCK);
 use POSIX qw(:signal_h);
 use IO::Handle ();
@@ -12,7 +14,6 @@ use IO::Handle ();
 # are available.
 sub new {
        my ($class, $sig, $flags) = @_;
-       my $self = fields::new($class);
        my %signo = map {;
                my $cb = $sig->{$_};
                # SIGWINCH is 28 on FreeBSD, NetBSD, OpenBSD
@@ -22,6 +23,7 @@ sub new {
                };
                $num => $cb;
        } keys %$sig;
+       my $self = bless { sig => \%signo }, $class;
        my $io;
        my $fd = signalfd(-1, [keys %signo], $flags);
        if (defined $fd && $fd >= 0) {
@@ -35,9 +37,8 @@ sub new {
                $self->SUPER::new($io, EPOLLIN | EPOLLET);
        } else { # master main loop
                $self->{sock} = $io;
+               $self;
        }
-       $self->{sig} = \%signo;
-       $self;
 }
 
 # PublicInbox::Daemon in master main loop (blocking)
index 97e67d3029a60736a125cc7770b04c2dec77ba92..660fdc77a2b740b5db8d085561dbaae52e73efd2 100644 (file)
@@ -132,8 +132,8 @@ done_testing;
 
 package IMAPC;
 use strict;
-use base qw(PublicInbox::DS);
-use fields qw(step zin);
+use parent qw(PublicInbox::DS);
+# fields: step: state machine, zin: Zlib inflate context
 use PublicInbox::Syscall qw(EPOLLIN EPOLLOUT EPOLLONESHOT);
 use Errno qw(EAGAIN);
 # determines where we start event_step
@@ -207,26 +207,23 @@ sub event_step {
 
 sub new {
        my ($class, $io) = @_;
-       my $self = fields::new($class);
-
-       # wait for connect(), and maybe SSL_connect()
-       $self->SUPER::new($io, EPOLLOUT|EPOLLONESHOT);
+       my $self = bless { step => FIRST_STEP }, $class;
        if ($io->can('connect_SSL')) {
                $self->{wbuf} = [ \&connect_tls_step ];
        }
-       $self->{step} = FIRST_STEP;
-       $self;
+       # wait for connect(), and maybe SSL_connect()
+       $self->SUPER::new($io, EPOLLOUT|EPOLLONESHOT);
 }
 
 1;
 package IMAPCdeflate;
 use strict;
-use base qw(IMAPC); # parent doesn't work for fields
-use Hash::Util qw(unlock_hash); # dependency of fields for perl 5.10+, anyways
+our @ISA;
 use Compress::Raw::Zlib;
 use PublicInbox::IMAPdeflate;
 my %ZIN_OPT;
 BEGIN {
+       @ISA = qw(IMAPC);
        %ZIN_OPT = ( -WindowBits => -15, -AppendOutput => 1 );
        *write = \&PublicInbox::IMAPdeflate::write;
        *do_read = \&PublicInbox::IMAPdeflate::do_read;
@@ -236,7 +233,6 @@ sub enable {
        my ($class, $self) = @_;
        my ($in, $err) = Compress::Raw::Zlib::Inflate->new(%ZIN_OPT);
        die "Inflate->new failed: $err" if $err != Z_OK;
-       unlock_hash(%$self);
        bless $self, $class;
        $self->{zin} = $in;
 }