Date | Commit message (Collapse) |
|
The use of `substr' here an argument to `print' was causing Perl
to internally cache its target buffer. Since `syswrite()'
already offers a buffer offset arg and length limits, just use
`syswrite' directly. We were using autoflush anyways, so the
lack of buffering was of no concern performance-wise.
The target buffer could get to roughly ~10MB under some loads,
but it was usually a cold path and using memory which cannot be
released nor reused in other places.
note: IO::Handle::write uses `substr' internally, too;
so nothing would be gained using IO::Handle:write.
|
|
I'm thinking we can drop support for Linux <2.6.27 soonish and
just use EPOLL_CLOEXEC. Perl without signalfd (or
EVFILT_SIGNAL) is miserable, actually.
|
|
Since signalfd is often combined with our event loop, give it a
convenient API and reduce the code duplication required to use it.
EventLoop is replaced with ::event_loop to allow consistent
parameter passing and avoid needlessly passing the package name
on stack.
We also avoid exporting SFD_NONBLOCK since it's the only flag we
support. There's no sense in having the memory overhead of a
constant function when it's in cold code.
|
|
add_uniq_timer seems sufficient, and we'll drop the last
user of ::later (IMAP) and switch to unique timers.
|
|
While it doesn't look like $EXPMAP can be populated in
non-obvious ways via ->DESTROY, it still makes sense to keep it
close to some of our other code around cleanup to reduce
the likelyhood of subtle bugs in case semantics change..
|
|
A common pattern we use is to arm a timer once and prevent
it from being armed until it fires. We'll be using it more
to do polling for saved searches and imports.
|
|
The use of substr within IO::Handle->write may not be correct if
we have wide characters, so handle it ourselves.
bytes.pm usage is probably better fixed in PublicInbox::NNTP,
but the effort required is higher, so we'll just keep bytes in
DS for now.
|
|
Cut down on unnecessary imports of IO::Handle and
method lookup + dispatch overhead.
|
|
None of these fixes affect current public-inbox-* code, or even
normal uses of lei. However, lei users wanting to switch
between $HOME directories or use alternate store paths may
notice strange behavior and this fixes some of it.
We'll also loop to account for DESTROY callbacks inserting into
container objects and retry appropriately.
|
|
We use croak in several places but weren't importing it :x
|
|
Packing args into an arrayref is awkward and we may be using
this API more in lei.
|
|
The PublicInbox::Eml (and previously Email::MIME) use of confess
was the primary (or only) culprit behind the lei2mail segfaults
fixed by commit 0795b0906cc81f40.
("ds: guard against stack-not-refcounted quirk of Perl 5").
We never care about a backtrace when dealing with Eml objects
anyways, so it was just a worthless waste of CPU cycles.
We can also drop confess in a few other places. Since we only
use Perl and Inline::C, users will never be without source
and can replace s/croak/Carp::confess/ on a per-callsite basis
to help report problems.
It's also possible to use PERL5OPT=-MCarp=verbose in the
environment though still potentially risky.
Link: https://public-inbox.org/meta/20210201082833.3293-1-e@80x24.org/
|
|
$_ at the top of a potentially deep stack below may cause
surprising behavior as I experienced with ExtSearchIdx. In the
future, we'll limit our $_ usage to easily-auditable bits (e.g.
map, grep, and small for loops)
|
|
The Perl 5 stack is weakly-referenced for performance reasons.
This means it's possible for items in the stack to be freed
while executing further down the stack.
In lei (and perhaps public-facing read-only daemons in the
future), we'll fork and call PublicInbox::DS->Reset in the child
process. This causes %DescriptorMap to be clobbered, allowing
the $DescriptorMap{$fd} arg to be freed inside the child
process.
When Carp::confess or Carp::longmess is called to generate a
backtrace, it may access the @DB::args array. This array access
is not protected by reference counting and is known to cause
segfaults and other weird errors.
While the caller of an unnecessary Carp::confess may be
eliminated in a future commit, we can't guarantee our
dependencies will be free of @DB::args access attempts
in the future.
So guard against this Perl 5 quirmk by defensively bumping the
refcount of any object we call ->event_step on.
cf. https://rt.perl.org/Public/Bug/Display.html?id=131046
https://github.com/Perl/perl5/issues/15928
|
|
This lets us call dwaitpid long before a process exits
and not have to wait around for it.
This is advantageous for lei where we can run dwaitpid on the
pager as soon as we spawn it, instead of waiting for a client
socket to go away on DESTROY.
|
|
This may help ensure DESTROY callbacks will see in_loop
before the others.
|
|
Objects with DESTROY callbacks get propagated to children, so we
must be careful to not invoke waitpid from children on their
sibling processes. Only parents (and their parents...) can reap
child processes.
|
|
This simplifies our code and provides a more consistent API for
error handling. PublicInbox::DS can be loaded nowadays on all
*BSDs and Linux distros easily without extra packages to
install.
The downside is possibly increased startup time, but it's
probably not as a big problem with lei being a daemon
(and -mda possibly following suite).
|
|
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.
|
|
More importantly, make it easier-to-find the sub by avoiding
runtime manipulation of subroutine names. There's no point in
avoiding a potential call to _InitPoller in EventLoop since
entering EventLoop is rare.
On the contrary, PublicInbox::DS->new is called often and this
change to avoid entering _InitPoller there may have more
benefits (which may still be unmeasurable).
|
|
Apparently they happen (triggered by my -imapd instance), so
bail out by closing the underlying socket rather than stopping
the event loop and daemon process.
|
|
Oops :x
|
|
While Perl implements tail recursion via `goto' which allows
avoiding warnings on deep recursion. It doesn't (as of 5.28)
optimize the speed of such dispatches, though it may reduce
ephemeral memory usage.
Make the code less alien to hackers coming from other languages
by using normal subroutine dispatch. It's actually slightly
faster in micro benchmarks due to the complexity of `goto &NAME'.
|
|
It doesn't seem necessary, since we won't call dwaitpid()
until we see an EOF.
|
|
We should not enqueue reap_pids() to run more than once per
EventLoop iteration. We'll start reformatting reap_pids
to tabs, too, since we're no longer Danga::Socket.
We should also be able to remove timer usage for reaping
down-the-line once we stop abusing dwaitpid() in -watch.
|
|
This allows callers to avoid creating expensive closures.
We no longer pass the `$now' value to callers, as none of
the callers used it.
|
|
We can avoid synchronous `waitpid(-1, 0)' and save a process
when simultaneously watching Maildirs.
One DS bug is fixed: ->Reset needs to clear the DS $in_loop flag
in forked children so dwaitpid() fails and allows git processes
to be reaped synchronously. TestCommon also calls DS->Reset
when spawning new processes, since t/imapd.t uses DS->EventLoop
while waiting on -watch to write.
|
|
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.
|
|
This quiets warnings from IMAP::fetch_blob (called via
long_response) failing to access `$self->{ibx}->git'
because ->{ibx} gets deleted by IMAP::close.
|
|
Doing a ref($obj) string comparison ties us to IO::Socket::SSL
(and OpenSSL) In the future, we may support GnuTLS or other TLS
implementations. This was already done in the IMAP code.
|
|
Dikshunarees R gude!
|
|
The "perlio" layer doesn't do read(2) syscalls over 8192 bytes
at the moment, and binmode($fh, ':unix') leaks[1]. So use
sysseek and sysread for now, since I can't see retaining
compatibility with PerlIO::scalar being worth the trouble.
[1] http://nntp.perl.org/group/perl.perl5.porters/256918
|
|
We want to be able to inject existing file handles + offsets and
even lengths into this in the future, without going through the
->getline interface[1]
We also switch to using a 64K buffer size since we can safely
discard whatever got truncated on write and full writes can help
negotiate a larger TCP window for high-latency, high-bandwidth
links.
While we're at it, make it obvious that we're using O_APPEND for
our tmpfile() interface so we can seek freely for reading while
the writer always prints to the end of the file.
[1] the getline interface for serving static files may result
in us buffering on-FS data into another temporary file,
which is a waste.
|
|
We can reduce the amount of small arrayrefs in memory
by flattening $EXPMAP. This forces us to properly clean
up references during deferred close handling, so NNTP
(and soon HTTP) connections no longer linger until expiry.
|
|
No reason to have an empty arrayref lying around when not
everybody needs it.
Re-indent the later-related subs since we're changing a
bunch of lines, anyways.
|
|
No need to create an arrayref until we need it, and fix up a
comment while we're in the area. Some aesthetic changes while
we're at it:
- Rename $WaitPids to $wait_pids to make it clear this is
unique to our implementation and not in Danga::Socket.
- rewrite dwaitpid() to reduce indentation level
|
|
Another place we can delay creating arrays until needed.
|
|
We can rely on autovification to turn `undef' value of {wbuf}
into an arrayref.
Furthermore, "push" returns the (new) size of the array since at
least Perl 5.0 (I didn't look further back), so we can use that
return value instead of calling "scalar" again.
|
|
This does not affect our current code, but theoretically a
DESTROY callback could call PublicInbox::DS::close to enqueue
elements into the ToClose array. So take a similar strategy as
we do with other queues (e.g. $nextq) by swapping references to
arrays, rather than operating on the array itself.
Since close operations are relatively rare, we can rely on
auto-vivification via "push" ops to create the array on an
as-needed basis.
Since we're in the area, clean up the PostLoopCallback
invocation to use the ternary operator rather than a confusing
(to me) combination of statements.
Finally, add a prototype to strengthen compile-time checking,
and move it in front of our only caller to make use of
the prototype.
|
|
It doesn't seem needed at the moment, and we can re-add it
in the future if needed.
|
|
Inbox.pm accessing the $in_loop variable directly raises
warnings when Inbox is loaded without DS.
|
|
The class parameter is pointless, especially for an internal
sub which only has one external caller in a test. Add a sub
prototype while we're at it to get some compile time checking.
|
|
"fileno(undef)" already dies under "use strict", so there's no
need to check for it ourselves. As far as "fileno($closed_io)"
or "fileno($fake_io)" goes, we'll let epoll_ctl detect the
error, instead.
Our design should make DescriptorMap entries impossible to clobber,
so make it fatal via confess in case it does happen, because
inadvertantly clobbering a FD would be very bad. While we're at
it, remove a redundant return statement and rely on implicit
returns.
|
|
Danga::Socket 1.62 was released a few months back and
the maintainer indicated it would be the last release.
We've diverged significantly in incompatible ways...
While most of this should've already been documented in
commit messages, putting it all into one document could
make it easier-to-digest.
It's also a strange design for anybody used to conventional
event loops. Maybe this is an unconventional project :P
|
|
There's a bunch of leftover "require" and "use" statements we no
longer need and can get rid of, along with some excessive
imports via "use".
IO::Handle usage isn't always obvious, so add comments
describing why a package loads it. Along the same lines,
document the tmpdir support as the reason we depend on
File::Temp 0.19, even though every Perl 5.10.1+ user has it.
While we're at it, favor "use" over "require", since it it gives
us extra compile-time checking.
|
|
HTTP::getline_pull and NNTP::long_step will both populate {wbuf}
manually to avoid recursion, so we need to account for an
empty-but-present {wbuf} while dispatching msg_more().
|
|
We need to use $PublicInbox::DS::in_loop instead of ::running().
The latter is not valid for systems with signalfd or kqueue and
is now gone, completely.
Not needing periodic cleanups at all to deal with unlinked pack
indices will be a tougher task...
|
|
We'll be supporting idle timeout for the HTTP code in the
future to deal directly with Internet-exposed clients w/o
Varnish or nginx.
|
|
EvCleanup only existed since Danga::Socket was a separate
component, and cleanup code belongs with the event loop.
|
|
I haven't noticed this being a problem in practice, but
be consistent with the rest of the singleton stuff.
Since we always call Reset() at load time, only do
initialization in that sub and not at declaration.
|