From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id DD67A1F5AE for ; Thu, 2 Jul 2020 03:32:56 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] inboxidle: avoid per-inbox anonymous subs Date: Thu, 2 Jul 2020 03:32:56 +0000 Message-Id: <20200702033256.15643-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Anonymous subs cost over 5K each on x86-64. So prefer the less-recommended-but-still-documented way of using Linux::Inotify2::watch to register watchers. This also updates FakeInotify to detect modifications correctly when used on systems with neither IO::KQueue nor Linux::Inotify2. --- lib/PublicInbox/DirIdle.pm | 13 +++++-- lib/PublicInbox/FakeInotify.pm | 70 +++++++++++++++++----------------- lib/PublicInbox/InboxIdle.pm | 19 +++++++-- lib/PublicInbox/KQNotify.pm | 36 ++++++++++------- t/fake_inotify.t | 18 ++++----- t/kqnotify.t | 12 ++---- 6 files changed, 92 insertions(+), 76 deletions(-) diff --git a/lib/PublicInbox/DirIdle.pm b/lib/PublicInbox/DirIdle.pm index fbbc9531a20..2915fd6e2da 100644 --- a/lib/PublicInbox/DirIdle.pm +++ b/lib/PublicInbox/DirIdle.pm @@ -23,7 +23,7 @@ if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) { sub new { my ($class, $dirs, $cb) = @_; - my $self = bless {}, $class; + my $self = bless { cb => $cb }, $class; my $inot; if ($ino_cls) { $inot = $ino_cls->new or die "E: $ino_cls->new: $!"; @@ -35,15 +35,20 @@ sub new { } # Linux::Inotify2->watch or similar - $inot->watch($_, $MAIL_IN, $cb) for @$dirs; + $inot->watch($_, $MAIL_IN) for @$dirs; $self->{inot} = $inot; + PublicInbox::FakeInotify::poll_once($self) if !$ino_cls; $self; } sub event_step { my ($self) = @_; - eval { $self->{inot}->poll }; # Linux::Inotify2::poll - warn "$self->{inot}->poll err: $@\n" if $@; + my $cb = $self->{cb}; + eval { + my @events = $self->{inot}->read; # Linux::Inotify2::poll + $cb->($_) for @events; + }; + warn "$self->{inot}->read err: $@\n" if $@; } 1; diff --git a/lib/PublicInbox/FakeInotify.pm b/lib/PublicInbox/FakeInotify.pm index debd2d39ae5..9275861368a 100644 --- a/lib/PublicInbox/FakeInotify.pm +++ b/lib/PublicInbox/FakeInotify.pm @@ -7,71 +7,66 @@ package PublicInbox::FakeInotify; use strict; use Time::HiRes qw(stat); use PublicInbox::DS; -my $IN_CLOSE = 0x08 | 0x10; # match Linux inotify +sub IN_MODIFY () { 0x02 } # match Linux inotify # my $IN_MOVED_TO = 0x80; # my $IN_CREATE = 0x100; sub MOVED_TO_OR_CREATE () { 0x80 | 0x100 } my $poll_intvl = 2; # same as Filesys::Notify::Simple -sub poll_once { - my ($self) = @_; - eval { $self->poll }; - warn "E: FakeInotify->poll: $@\n" if $@; - PublicInbox::DS::add_timer($poll_intvl, \&poll_once, $self); -} - -sub new { - my $self = bless { watch => {} }, __PACKAGE__; - PublicInbox::DS::add_timer($poll_intvl, \&poll_once, $self); - $self; -} +sub new { bless { watch => {} }, __PACKAGE__ } # behaves like Linux::Inotify2->watch sub watch { - my ($self, $path, $mask, $cb) = @_; + my ($self, $path, $mask) = @_; my @st = stat($path) or return; my $k = "$path\0$mask"; - $self->{watch}->{$k} = [ $st[10], $cb ]; # 10 - ctime + $self->{watch}->{$k} = $st[10]; # 10 - ctime bless [ $self->{watch}, $k ], 'PublicInbox::FakeInotify::Watch'; } sub on_new_files ($$$$) { - my ($dh, $cb, $path, $old_ctime) = @_; + my ($events, $dh, $path, $old_ctime) = @_; while (defined(my $base = readdir($dh))) { next if $base =~ /\A\.\.?\z/; my $full = "$path/$base"; my @st = stat($full); if (@st && $st[10] > $old_ctime) { - bless \$full, 'PublicInbox::FakeInotify::Event'; - eval { $cb->(\$full) }; + push @$events, + bless(\$full, 'PublicInbox::FakeInotify::Event') } } } -# behaves like non-blocking Linux::Inotify2->poll -sub poll { +# behaves like non-blocking Linux::Inotify2->read +sub read { my ($self) = @_; - my $watch = $self->{watch} or return; + my $watch = $self->{watch} or return (); + my $events = []; for my $x (keys %$watch) { my ($path, $mask) = split(/\0/, $x, 2); my @now = stat($path) or next; - my $prv = $watch->{$x}; - my $cb = $prv->[-1]; - my $old_ctime = $prv->[0]; - if ($old_ctime != $now[10]) { - if (($mask & $IN_CLOSE) == $IN_CLOSE) { - eval { $cb->() }; - } elsif ($mask & MOVED_TO_OR_CREATE) { - opendir(my $dh, $path) or do { - warn "W: opendir $path: $!\n"; - next; - }; - on_new_files($dh, $cb, $path, $old_ctime); - } + my $old_ctime = $watch->{$x}; + $watch->{$x} = $now[10]; + next if $old_ctime == $now[10]; + if ($mask & IN_MODIFY) { + push @$events, + bless(\$path, 'PublicInbox::FakeInotify::Event') + } elsif ($mask & MOVED_TO_OR_CREATE) { + opendir(my $dh, $path) or do { + warn "W: opendir $path: $!\n"; + next; + }; + on_new_files($events, $dh, $path, $old_ctime); } - @$prv = ($now[10], $cb); } + @$events; +} + +sub poll_once { + my ($obj) = @_; + $obj->event_step; # PublicInbox::InboxIdle::event_step + PublicInbox::DS::add_timer($poll_intvl, \&poll_once, $obj); } package PublicInbox::FakeInotify::Watch; @@ -82,6 +77,11 @@ sub cancel { delete $self->[0]->{$self->[1]}; } +sub name { + my ($self) = @_; + (split(/\0/, $self->[1], 2))[0]; +} + package PublicInbox::FakeInotify::Event; use strict; diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm index 59cb833fd5a..bdb30284ac0 100644 --- a/lib/PublicInbox/InboxIdle.pm +++ b/lib/PublicInbox/InboxIdle.pm @@ -36,12 +36,13 @@ sub in2_arm ($$) { # PublicInbox::Config::each_inbox callback $ibx->{unlock_subs} and die "BUG: $dir->{unlock_subs} should not exist"; $ibx->{unlock_subs} = $old_ibx->{unlock_subs}; - $cur->[1]->cancel; + $cur->[1]->cancel; # Linux::Inotify2::Watch::cancel } $cur->[0] = $ibx; my $lock = "$dir/".($ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock'); - $cur->[1] = $inot->watch($lock, $IN_MODIFY, sub { $ibx->on_unlock }); + my $w = $cur->[1] = $inot->watch($lock, $IN_MODIFY); + $self->{on_unlock}->{$w->name} = $ibx; # TODO: detect deleted packs (and possibly other files) } @@ -65,14 +66,24 @@ sub new { } $self->{inot} = $inot; $self->{pathmap} = {}; # inboxdir => [ ibx, watch1, watch2, watch3...] + $self->{on_unlock} = {}; # lock path => ibx refresh($self, $pi_config); + PublicInbox::FakeInotify::poll_once($self) if !$ino_cls; $self; } sub event_step { my ($self) = @_; - eval { $self->{inot}->poll }; # Linux::Inotify2::poll - warn "$self->{inot}->poll err: $@\n" if $@; + eval { + my @events = $self->{inot}->read; # Linux::Inotify2::read + my $on_unlock = $self->{on_unlock}; + for my $ev (@events) { + if (my $ibx = $on_unlock->{$ev->fullname}) { + $ibx->on_unlock; + } + } + }; + warn "{inot}->read err: $@\n" if $@; } # for graceful shutdown in PublicInbox::Daemon, diff --git a/lib/PublicInbox/KQNotify.pm b/lib/PublicInbox/KQNotify.pm index 9673b44290a..2305daa785b 100644 --- a/lib/PublicInbox/KQNotify.pm +++ b/lib/PublicInbox/KQNotify.pm @@ -19,16 +19,17 @@ sub new { } sub watch { - my ($self, $path, $mask, $cb) = @_; - my ($fh, $cls, @extra); + my ($self, $path, $mask) = @_; + my ($fh, $watch); if (-d $path) { opendir($fh, $path) or return; my @st = stat($fh); - @extra = ($path, $st[10]); # 10: ctime - $cls = 'PublicInbox::KQNotify::Watchdir'; + $watch = bless [ $fh, $path, $st[10] ], + 'PublicInbox::KQNotify::Watchdir'; } else { open($fh, '<', $path) or return; - $cls = 'PublicInbox::KQNotify::Watch'; + $watch = bless [ $fh, $path ], + 'PublicInbox::KQNotify::Watch'; } my $ident = fileno($fh); $self->{dskq}->{kq}->EV_SET($ident, # ident @@ -37,11 +38,11 @@ sub watch { $mask, # fflags 0, 0); # data, udata if ($mask == NOTE_WRITE || $mask == MOVED_TO_OR_CREATE) { - $self->{watch}->{$ident} = [ $fh, $cb, @extra ]; + $self->{watch}->{$ident} = $watch; } else { die "TODO Not implemented: $mask"; } - bless \$fh, $cls; + $watch; } # emulate Linux::Inotify::fileno @@ -56,33 +57,40 @@ sub on_overflow {} sub blocking {} # behave like Linux::Inotify2::poll -sub poll { +sub read { my ($self) = @_; my @kevents = $self->{dskq}->{kq}->kevent(0); + my $events = []; for my $kev (@kevents) { my $ident = $kev->[KQ_IDENT]; my $mask = $kev->[KQ_FFLAGS]; - my ($dh, $cb, $path, $old_ctime) = @{$self->{watch}->{$ident}}; - if (!defined($path) && ($mask & NOTE_WRITE) == NOTE_WRITE) { - eval { $cb->() }; + my ($dh, $path, $old_ctime) = @{$self->{watch}->{$ident}}; + if (!defined($old_ctime)) { + push @$events, + bless(\$path, 'PublicInbox::FakeInotify::Event') } elsif ($mask & MOVED_TO_OR_CREATE) { my @new_st = stat($path) or next; $self->{watch}->{$ident}->[3] = $new_st[10]; # ctime rewinddir($dh); - PublicInbox::FakeInotify::on_new_files($dh, $cb, + PublicInbox::FakeInotify::on_new_files($events, $dh, $path, $old_ctime); } } + @$events; } package PublicInbox::KQNotify::Watch; use strict; -sub cancel { close ${$_[0]} or die "close: $!" } +sub name { $_[0]->[1] } + +sub cancel { close $_[0]->[0] or die "close: $!" } package PublicInbox::KQNotify::Watchdir; use strict; -sub cancel { closedir ${$_[0]} or die "closedir: $!" } +sub name { $_[0]->[1] } + +sub cancel { closedir $_[0]->[0] or die "closedir: $!" } 1; diff --git a/t/fake_inotify.t b/t/fake_inotify.t index f0db0cb58ec..11dac117f0d 100644 --- a/t/fake_inotify.t +++ b/t/fake_inotify.t @@ -16,29 +16,25 @@ close $fh or BAIL_OUT "close: $!"; my $fi = PublicInbox::FakeInotify->new; my $mask = PublicInbox::FakeInotify::MOVED_TO_OR_CREATE(); -my $hit = []; -my $cb = sub { push @$hit, map { $_->fullname } @_ }; -my $w = $fi->watch("$tmpdir/new", $mask, $cb); +my $w = $fi->watch("$tmpdir/new", $mask); select undef, undef, undef, $MIN_FS_TICK; rename("$tmpdir/tst", "$tmpdir/new/tst") or BAIL_OUT "rename: $!"; -$fi->poll; -is_deeply($hit, ["$tmpdir/new/tst"], 'rename(2) detected'); +my @events = map { $_->fullname } $fi->read; +is_deeply(\@events, ["$tmpdir/new/tst"], 'rename(2) detected'); -@$hit = (); select undef, undef, undef, $MIN_FS_TICK; open $fh, '>', "$tmpdir/tst" or BAIL_OUT "open: $!"; close $fh or BAIL_OUT "close: $!"; link("$tmpdir/tst", "$tmpdir/new/link") or BAIL_OUT "link: $!"; -$fi->poll; -is_deeply($hit, ["$tmpdir/new/link"], 'link(2) detected'); +@events = map { $_->fullname } $fi->read; +is_deeply(\@events, ["$tmpdir/new/link"], 'link(2) detected'); $w->cancel; -@$hit = (); select undef, undef, undef, $MIN_FS_TICK; link("$tmpdir/new/tst", "$tmpdir/new/link2") or BAIL_OUT "link: $!"; -$fi->poll; -is_deeply($hit, [], 'link(2) not detected after cancel'); +@events = map { $_->fullname } $fi->read; +is_deeply(\@events, [], 'link(2) not detected after cancel'); PublicInbox::DS->Reset; diff --git a/t/kqnotify.t b/t/kqnotify.t index b3414b8ae33..c3557d3e25b 100644 --- a/t/kqnotify.t +++ b/t/kqnotify.t @@ -17,25 +17,21 @@ close $fh or BAIL_OUT "close: $!"; my $kqn = PublicInbox::KQNotify->new; my $mask = PublicInbox::KQNotify::MOVED_TO_OR_CREATE(); -my $hit = []; -my $cb = sub { push @$hit, map { $_->fullname } @_ }; -my $w = $kqn->watch("$tmpdir/new", $mask, $cb); +my $w = $kqn->watch("$tmpdir/new", $mask); rename("$tmpdir/tst", "$tmpdir/new/tst") or BAIL_OUT "rename: $!"; -$kqn->poll; +my $hit = [ map { $_->fullname } $kqn->read ]; is_deeply($hit, ["$tmpdir/new/tst"], 'rename(2) detected (via NOTE_EXTEND)'); -@$hit = (); open $fh, '>', "$tmpdir/tst" or BAIL_OUT "open: $!"; close $fh or BAIL_OUT "close: $!"; link("$tmpdir/tst", "$tmpdir/new/link") or BAIL_OUT "link: $!"; -$kqn->poll; +$hit = [ grep m!/link$!, map { $_->fullname } $kqn->read ]; is_deeply($hit, ["$tmpdir/new/link"], 'link(2) detected (via NOTE_WRITE)'); $w->cancel; -@$hit = (); link("$tmpdir/new/tst", "$tmpdir/new/link2") or BAIL_OUT "link: $!"; -$kqn->poll; +$hit = [ map { $_->fullname } $kqn->read ]; is_deeply($hit, [], 'link(2) not detected after cancel'); done_testing;