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/6] ds: object lifetime shortening
@ 2023-10-31 20:42  7% Eric Wong
  2023-10-31 20:42  5% ` [PATCH 2/6] ds: do not defer close Eric Wong
  0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
  To: meta

1 and 2 do the bulk of the work
3 and 4 are things I noticed when working on 2
5 is a fix while preparing for 6
6 is something I've wanted to do for a long time.

Anyways, the code reductions always make me happy.

Eric Wong (6):
  ds: next_tick: shorten object lifetimes
  ds: do not defer close
  ds: move maxevents further down the stack
  watch: simplify DirIdle object cleanup
  pop3: use SSL_shutdown(3ssl) if appropriate
  ds: make ->close behave like CORE::close

 lib/PublicInbox/CidxComm.pm |   2 +-
 lib/PublicInbox/CidxLogP.pm |   2 +-
 lib/PublicInbox/DS.pm       | 143 ++++++++++++------------------------
 lib/PublicInbox/DSKQXS.pm   |   3 +-
 lib/PublicInbox/DSPoll.pm   |   2 +-
 lib/PublicInbox/Epoll.pm    |   5 +-
 lib/PublicInbox/POP3.pm     |   2 +-
 lib/PublicInbox/Select.pm   |   2 +-
 lib/PublicInbox/Watch.pm    |  11 +--
 t/ds-poll.t                 |  14 ++--
 t/epoll.t                   |   4 +-
 11 files changed, 71 insertions(+), 119 deletions(-)

^ permalink raw reply	[relevance 7%]

* [PATCH 2/6] ds: do not defer close
  2023-10-31 20:42  7% [PATCH 0/6] ds: object lifetime shortening Eric Wong
@ 2023-10-31 20:42  5% ` Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
  To: meta

We can map all integer FDs to Perl objects once ->ep_wait returns,
so there's no need to play tricks elsewhere to ensure FDs can
be mapped to objects within the same event loop iteration.
---
 lib/PublicInbox/DS.pm | 67 ++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 45 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 708fe14d..b30e9db6 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -39,7 +39,7 @@ our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer);
 
 my $nextq; # queue for next_tick
 my $reap_armed;
-my $ToClose; # sockets to close when event loop is done
+my @active; # FDs (or objs) returned by epoll/kqueue
 our (%AWAIT_PIDS, # pid => [ $callback, @args ]
 	$cur_runq, # only set inside next_tick
      %DescriptorMap,             # fd (num) -> PublicInbox::DS object
@@ -81,10 +81,11 @@ sub Reset {
 
 		# we may be called from an *atfork_child inside next_tick:
 		@$cur_runq = () if $cur_runq;
-		$nextq = $ToClose = undef; # may call ep_del
+		@active = ();
+		$nextq = undef; # may call ep_del
 		%AWAIT_PIDS = ();
 	} while (@Timers || $nextq || keys(%AWAIT_PIDS) ||
-		$ToClose || keys(%DescriptorMap) ||
+		@active || keys(%DescriptorMap) ||
 		@post_loop_do || keys(%UniqTimer) ||
 		scalar(@{$cur_runq // []})); # do not vivify cur_runq
 
@@ -149,7 +150,7 @@ sub next_tick () {
 sub RunTimers {
 	next_tick();
 
-	return (($nextq || $ToClose) ? 0 : $loop_timeout) unless @Timers;
+	return ($nextq ? 0 : $loop_timeout) unless @Timers;
 
 	my $now = now();
 
@@ -161,7 +162,7 @@ sub RunTimers {
 	}
 
 	# timers may enqueue into nextq:
-	return 0 if ($nextq || $ToClose);
+	return 0 if $nextq;
 
 	return $loop_timeout unless @Timers;
 
@@ -236,18 +237,6 @@ sub close_non_busy () {
 # for pushed-back data, and close pending connections.  returns 1
 # if event loop should continue, or 0 to shut it all down.
 sub PostEventLoop () {
-	# 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)
-	if (my $close_now = $ToClose) {
-		$ToClose = undef; # will be autovivified on push
-		@$close_now = map { fileno($_) } @$close_now;
-
-		# ->DESTROY methods may populate ToClose
-		delete @DescriptorMap{@$close_now};
-	}
-
 	# by default we keep running, unless a postloop callback cancels it
 	@post_loop_do ? $post_loop_do[0]->(@post_loop_do[1..$#post_loop_do]) : 1
 }
@@ -295,21 +284,16 @@ sub event_loop (;$$) {
 	}
 	$_[0] = $sigfd = $sig = undef; # $_[0] == sig
 	local $in_loop = 1;
-	my @events;
 	do {
 		my $timeout = RunTimers();
 
-		# get up to 1000 events
-		$Poller->ep_wait(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.
-
-			# guard stack-not-refcounted w/ Carp + @DB::args
-			my $obj = $DescriptorMap{$fd};
+		# get up to 1000 FDs representing events
+		$Poller->ep_wait(1000, $timeout, \@active);
+
+		# map all FDs to their associated Perl object
+		@active = @DescriptorMap{@active};
+
+		while (my $obj = shift @active) {
 			$obj->event_step;
 		}
 	} while (PostEventLoop());
@@ -390,24 +374,17 @@ sub close {
     # preventing the object from being destroyed
     delete $self->{wbuf};
 
-    # if we're using epoll, we have to remove this from our epoll fd so we stop getting
-    # notifications about it
-    $Poller->ep_del($sock) and croak("EPOLL_CTL_DEL($self/$sock): $!");
-
-    # we explicitly don't delete from DescriptorMap here until we
-    # actually close the socket, as we might be in the middle of
-    # processing an epoll_wait/etc that returned hundreds of fds, one
-    # of which is not yet processed and is what we're closing.  if we
-    # keep it in DescriptorMap, then the event harnesses can just
-    # looked at $pob->{sock} == undef and ignore it.  but if it's an
-    # un-accounted for fd, then it (understandably) freak out a bit
-    # and emit warnings, thinking their state got off.
+    delete $DescriptorMap{fileno($sock)};
 
-    # defer closing the actual socket until the event loop is done
-    # processing this round of events.  (otherwise we might reuse fds)
-    push @$ToClose, $sock; # autovivifies $ToClose
+    # if we're using epoll, we have to remove this from our epoll fd so we
+    # stop getting notifications about it
+    $Poller->ep_del($sock) and croak("EPOLL_CTL_DEL($self/$sock): $!");
+    # let refcount drop to zero..
 
-    return 0;
+    # FIXME this is the opposite of CORE::close return value
+    # TODO: callers need to be updated to expect true on success like
+    # CORE::close (and false otherwise)
+    0;
 }
 
 # portable, non-thread-safe sendfile emulation (no pread, yet)

^ permalink raw reply related	[relevance 5%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-10-31 20:42  7% [PATCH 0/6] ds: object lifetime shortening Eric Wong
2023-10-31 20:42  5% ` [PATCH 2/6] ds: do not defer close 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).