]> Sergey Matveev's repositories - public-inbox.git/commitdiff
ds: flatten + reuse @events, epoll_wait style fixes
authorEric Wong <e@80x24.org>
Sun, 27 Dec 2020 02:53:07 +0000 (02:53 +0000)
committerEric Wong <e@80x24.org>
Mon, 28 Dec 2020 23:19:50 +0000 (23:19 +0000)
Consistently returning the equivalent of pollfd.revents in a
portable manner was never worth the effort for us, as we use the
same ->event_step callback regardless of POLLIN/POLLOUT/POLLHUP.

Being a Perl, @events knows it size and we don't have to return
a maximum index for the caller to iterate on.

We can also avoid redundant integer coercion ("+0") since we
ensure everything is an IV in other places.

Finally, vec() is preferable to ("\0" x $size) for resizing
buffers because it only needs to write the extended portion
and not overwrite the entire buffer.

lib/PublicInbox/DS.pm
lib/PublicInbox/DSKQXS.pm
lib/PublicInbox/DSPoll.pm
lib/PublicInbox/Syscall.pm
t/ds-poll.t
t/epoll.t

index 12df59192cc96f890626795b32d993d0fe372029..97a6f6efc721acdecfd4c0d3c447fa3f08f20f25 100644 (file)
@@ -87,9 +87,7 @@ A timeout of 0 (zero) means poll forever. A timeout of -1 means poll and return
 immediately.
 
 =cut
-sub SetLoopTimeout {
-    return $LoopTimeout = $_[1] + 0;
-}
+sub SetLoopTimeout { $LoopTimeout = $_[1] + 0 }
 
 =head2 C<< PublicInbox::DS::add_timer( $seconds, $coderef, $arg) >>
 
@@ -200,12 +198,7 @@ sub RunTimers {
     my $timeout = int(($Timers[0][0] - $now) * 1000) + 1;
 
     # -1 is an infinite timeout, so prefer a real timeout
-    return $timeout     if $LoopTimeout == -1;
-
-    # otherwise pick the lower of our regular timeout and time until
-    # the next timer
-    return $LoopTimeout if $LoopTimeout < $timeout;
-    return $timeout;
+    ($LoopTimeout < 0 || $LoopTimeout >= $timeout) ? $timeout : $LoopTimeout;
 }
 
 # We can't use waitpid(-1) safely here since it can hit ``, system(),
@@ -261,19 +254,18 @@ sub PostEventLoop () {
 sub EventLoop {
     $Epoll //= _InitPoller();
     local $in_loop = 1;
+    my @events;
     do {
-        my @events;
-        my $i;
         my $timeout = RunTimers();
 
         # get up to 1000 events
-        my $evcount = epoll_wait($Epoll, 1000, $timeout, \@events);
-        for ($i=0; $i<$evcount; $i++) {
+        epoll_wait($Epoll, 1000, $timeout, \@events);
+        for my $fd (@events) {
             # it's possible epoll_wait returned many events, including some at the end
             # that ones in the front triggered unregister-interest actions.  if we
             # can't find the %sock entry, it's because we're no longer interested
             # in that event.
-            $DescriptorMap{$events[$i]->[0]}->event_step;
+            $DescriptorMap{$fd}->event_step;
         }
     } while (PostEventLoop());
     _run_later();
index d1d3fe60d4a771cdcbe648b95f4c89ad646af1c2..aa2c91680e7dd2b6ab3ba283e76472358fc86def 100644 (file)
@@ -134,7 +134,7 @@ sub epoll_wait {
                }
        }
        # caller only cares for $events[$i]->[0]
-       scalar(@$events);
+       $_ = $_->[0] for @$events;
 }
 
 # kqueue is close-on-fork (not exec), so we must not close it
index 1d9b51d9f267b0570517b3eda0d7faa47d016ffb..a218f69563f04b944f046c514f4b9ce237c655b3 100644 (file)
@@ -45,14 +45,13 @@ sub epoll_wait {
                        my $fd = $pset[$i++];
                        my $revents = $pset[$i++] or next;
                        delete($self->{$fd}) if $self->{$fd} & EPOLLONESHOT;
-                       push @$events, [ $fd ];
+                       push @$events, $fd;
                }
                my $nevents = scalar @$events;
                if ($n != $nevents) {
                        warn "BUG? poll() returned $n, but got $nevents";
                }
        }
-       $n;
 }
 
 1;
index e4f00a2a2f736a995072dbac0db320ea49cb925e..c403f78ad209eac4c5c89f80c0545b17871e6619 100644 (file)
@@ -227,38 +227,46 @@ sub epoll_ctl_mod8 {
 our $epoll_wait_events;
 our $epoll_wait_size = 0;
 sub epoll_wait_mod4 {
-    # resize our static buffer if requested size is bigger than we've ever done
-    if ($_[1] > $epoll_wait_size) {
-        $epoll_wait_size = $_[1];
-        $epoll_wait_events = "\0" x 12 x $epoll_wait_size;
-    }
-    my $ct = syscall($SYS_epoll_wait, $_[0]+0, $epoll_wait_events, $_[1]+0, $_[2]+0);
-    for (0..$ct-1) {
-        @{$_[3]->[$_]}[1,0] = unpack("LL", substr($epoll_wait_events, 12*$_, 8));
-    }
-    return $ct;
+       my ($epfd, $maxevents, $timeout_msec, $events) = @_;
+       # resize our static buffer if maxevents bigger than we've ever done
+       if ($maxevents > $epoll_wait_size) {
+               $epoll_wait_size = $maxevents;
+               vec($epoll_wait_events, $maxevents * 12 * 8 - 1, 1) = 0;
+       }
+       @$events = ();
+       my $ct = syscall($SYS_epoll_wait, $epfd, $epoll_wait_events,
+                       $maxevents, $timeout_msec);
+       for (0..$ct - 1) {
+               # 12-byte struct epoll_event
+               # 4 bytes uint32_t events mask (skipped, useless to us)
+               # 8 bytes: epoll_data_t union (first 4 bytes are the fd)
+               # So we skip the first 4 bytes and take the middle 4:
+               $events->[$_] = unpack('L', substr($epoll_wait_events,
+                                                       12 * $_ + 4, 4));
+       }
 }
 
 sub epoll_wait_mod8 {
-    # resize our static buffer if requested size is bigger than we've ever done
-    if ($_[1] > $epoll_wait_size) {
-        $epoll_wait_size = $_[1];
-        $epoll_wait_events = "\0" x 16 x $epoll_wait_size;
-    }
-    my $ct;
-    if ($no_deprecated) {
-        $ct = syscall($SYS_epoll_wait, $_[0]+0, $epoll_wait_events, $_[1]+0, $_[2]+0, undef);
-    } else {
-        $ct = syscall($SYS_epoll_wait, $_[0]+0, $epoll_wait_events, $_[1]+0, $_[2]+0);
-    }
-    for (0..$ct-1) {
-        # 16 byte epoll_event structs, with format:
-        #    4 byte mask [idx 1]
-        #    4 byte padding (we put it into idx 2, useless)
-        #    8 byte data (first 4 bytes are fd, into idx 0)
-        @{$_[3]->[$_]}[1,2,0] = unpack("LLL", substr($epoll_wait_events, 16*$_, 12));
-    }
-    return $ct;
+       my ($epfd, $maxevents, $timeout_msec, $events) = @_;
+
+       # resize our static buffer if maxevents bigger than we've ever done
+       if ($maxevents > $epoll_wait_size) {
+               $epoll_wait_size = $maxevents;
+               vec($epoll_wait_events, $maxevents * 16 * 8 - 1, 1) = 0;
+       }
+       @$events = ();
+       my $ct = syscall($SYS_epoll_wait, $epfd, $epoll_wait_events,
+                       $maxevents, $timeout_msec,
+                       $no_deprecated ? undef : ());
+       for (0..$ct - 1) {
+               # 16-byte struct epoll_event
+               # 4 bytes uint32_t events mask (skipped, useless to us)
+               # 4 bytes padding (skipped, useless)
+               # 8 bytes epoll_data_t union (first 4 bytes are the fd)
+               # So skip the first 8 bytes, take 4, and ignore the last 4:
+               $events->[$_] = unpack('L', substr($epoll_wait_events,
+                                                       16 * $_ + 8, 4));
+       }
 }
 
 sub signalfd ($$$) {
index 3771059bb6ecb32edf30fcac17d359005a079337..0ee57b692a600aef2729eea6375ccf477a5321fb 100644 (file)
@@ -16,35 +16,35 @@ pipe($r, $w) or die;
 pipe($x, $y) or die;
 is($p->epoll_ctl(EPOLL_CTL_ADD, fileno($r), EPOLLIN), 0, 'add EPOLLIN');
 my $events = [];
-my $n = $p->epoll_wait(9, 0, $events);
+$p->epoll_wait(9, 0, $events);
 is_deeply($events, [], 'no events set');
-is($n, 0, 'nothing ready, yet');
 is($p->epoll_ctl(EPOLL_CTL_ADD, fileno($w), EPOLLOUT|EPOLLONESHOT), 0,
        'add EPOLLOUT|EPOLLONESHOT');
-$n = $p->epoll_wait(9, -1, $events);
-is($n, 1, 'got POLLOUT event');
-is($events->[0]->[0], fileno($w), '$w ready');
+$p->epoll_wait(9, -1, $events);
+is(scalar(@$events), 1, 'got POLLOUT event');
+is($events->[0], fileno($w), '$w ready');
 
-$n = $p->epoll_wait(9, 0, $events);
-is($n, 0, 'nothing ready after oneshot');
+$p->epoll_wait(9, 0, $events);
+is(scalar(@$events), 0, 'nothing ready after oneshot');
 is_deeply($events, [], 'no events set after oneshot');
 
 syswrite($w, '1') == 1 or die;
 for my $t (0..1) {
-       $n = $p->epoll_wait(9, $t, $events);
-       is($events->[0]->[0], fileno($r), "level-trigger POLLIN ready #$t");
-       is($n, 1, "only event ready #$t");
+       $p->epoll_wait(9, $t, $events);
+       is($events->[0], fileno($r), "level-trigger POLLIN ready #$t");
+       is(scalar(@$events), 1, "only event ready #$t");
 }
 syswrite($y, '1') == 1 or die;
 is($p->epoll_ctl(EPOLL_CTL_ADD, fileno($x), EPOLLIN|EPOLLONESHOT), 0,
        'EPOLLIN|EPOLLONESHOT add');
-is($p->epoll_wait(9, -1, $events), 2, 'epoll_wait has 2 ready');
-my @fds = sort(map { $_->[0] } @$events);
+$p->epoll_wait(9, -1, $events);
+is(scalar @$events, 2, 'epoll_wait has 2 ready');
+my @fds = sort @$events;
 my @exp = sort((fileno($r), fileno($x)));
 is_deeply(\@fds, \@exp, 'got both ready FDs');
 
 is($p->epoll_ctl(EPOLL_CTL_DEL, fileno($r), 0), 0, 'EPOLL_CTL_DEL OK');
-$n = $p->epoll_wait(9, 0, $events);
-is($n, 0, 'nothing ready after EPOLL_CTL_DEL');
+$p->epoll_wait(9, 0, $events);
+is(scalar @$events, 0, 'nothing ready after EPOLL_CTL_DEL');
 
 done_testing;
index b47650e34bb21b472349048e0f3843cdb1fbd88d..a1e73e07387d8cd74e1813ab06d312d7f2a1ae8c 100644 (file)
--- a/t/epoll.t
+++ b/t/epoll.t
@@ -12,11 +12,11 @@ is(epoll_ctl($epfd, EPOLL_CTL_ADD, fileno($w), EPOLLOUT), 0,
     'epoll_ctl socket EPOLLOUT');
 
 my @events;
-is(epoll_wait($epfd, 100, 10000, \@events), 1, 'epoll_wait returns');
+epoll_wait($epfd, 100, 10000, \@events);
 is(scalar(@events), 1, 'got one event');
-is($events[0]->[0], fileno($w), 'got expected FD');
-is($events[0]->[1], EPOLLOUT, 'got expected event');
+is($events[0], fileno($w), 'got expected FD');
 close $w;
-is(epoll_wait($epfd, 100, 0, \@events), 0, 'epoll_wait timeout');
+epoll_wait($epfd, 100, 0, \@events);
+is(@events, 0, 'epoll_wait timeout');
 
 done_testing;