Huh, a few confusing test failures surface.

I experimented with EVFILT_USER around 2013 and I remember it was broken on macos. Looks like it still is?

I'm afraid I don't really remember the details. Possibly it only worked as a oneshot event, although it seems libevent uses it in multishot mode.

I experimented with EVFILT_USER around 2013 and I remember it was broken on macos. Looks like it still is?

I'm afraid I don't really remember the details. Possibly it only worked as a oneshot event, although it seems libevent uses it in multishot mode.

EVFILT_USER doesn't seem to exist in the macOS man pages anymore. Maybe there is indeed something wrong with EVFILT_USER.

Besides, it looks like EVFILT_USER is available on NetBSD since this commit.

Unfortunately, FreeBSD and NetBSD are not available with GitHub Actions, but something like cross-platform-actions may come to help.

Would it be sensible to try creating a zero-second one shot timer timeout instead?

Would it be sensible to try creating a zero-second one shot timer timeout instead?

I don't think this is the right path. In that way, we're unable to ensure a real-time interruption to kqueue/epoll/... cuz it needs to wait for the in-flight uv__io_poll() to finish or for the next round to begin.

Isn't it waiting for those anyways?

Isn't it waiting for those anyways?

No, it isn't. And I think that's why async.c was created, to allow other threads to emit real-time interruptions to the main thread, waking it up from the polling immediately. Please correct me if I'm missing something.

A zero second timer should wake it immediately too, though I won't claim to know if that is a useful alternative to the existing pipe

Kindly ping @bnoordhuis

After thinking it over, I'm somewhat disinclined to merge this. It's for niche platforms that I personally don't care about, the changes get intermingled with platform code that I do care about, and the real-world performance improvements aren't clear.

I could maybe be persuaded if it gave a really good bump on the async handles benchmarks (grep test/benchmark-list.h for "async") but no promises.

After thinking it over, I'm somewhat disinclined to merge this. It's for niche platforms that I personally don't care about, the changes get intermingled with platform code that I do care about, and the real-world performance improvements aren't clear.

I could maybe be persuaded if it gave a really good bump on the async handles benchmarks (grep test/benchmark-list.h for "async") but no promises.

From where I stand, well-testing can justify merging code of improvement. Besides, NetBSD may not catch your attention, but FreeBSD is the Tier-2 support of libuv (also the most popular *BSD system). So if we can run the test suite of libuv on FreeBSD and no errors occur, I'd say it is worth going along.

Let's see where other maintainers stand on this.

I've finally discovered the root cause that had been failing the tests on macOS before, uv__async_send() might get called before uv__io_poll() with multi-threads, which resulted in an ENOENT returned from kevent() and the process got aborted. I think we can resume this PR now! @bnoordhuis @vtjnash

There might be some code conflicts with #4400.

Some references of EVFILT_USER on various platforms:

• FreeBSD kqueue
• NetBSD kqueue
• DragonFlyBSD annotated tag v4.0.0rc created
• macOS swift-nio
• XNU kernel

Okay, it wasn't clear that EVFILT_USER was available on XNU. Makes more sense now that is clear it is available in the XNU sources, just not documented. Could you run the benchmark on this?

Okay, it wasn't clear that EVFILT_USER was available on XNU. Makes more sense now that is clear it is available in the XNU sources, just not documented. Could you run the benchmark on this?

Environment

 Model : Mac Studio (2022)
    OS : macOS 14.1.2
   CPU : Apple M1 Max, 10-core CPU with 8 performance cores and 2 efficiency cores
Memory : 32GB unified memory

Benchmark command

build/uv_run_benchmarks_a million_async

Benchmark results

Branch: libuv:v1.x

ok 1 - million_async
# 10,348,986 async events in 5.0 seconds (2,069,797/s, 1,048,576 unique handles seen)

Branch: panjf2000:kqueue-wake

ok 1 - million_async
# 12,977,383 async events in 5.0 seconds (2,595,476/s, 1,048,576 unique handles seen)

@vtjnash

Nice!

Any new thoughts on this? @bnoordhuis @vtjnash @saghul

Ping @bnoordhuis @vtjnash

I went with uv_once() cuz this approach can prevent multiple runtime detections from fork(2)

Using fork in a program linked against pthreads or that uses pthreads_once is not supported and not a legal program, so that doesn't really matter. The extra kqueue with this approach seems like a bit of extra complexity, but not enough to be an issue, so SGTM.

Is this ready to land? Once this lands I'll proceed with the release.

Just rebased the code from the mainline, nothing changed in this PR.

There appears to be a 1 second sleep in the code at startup that should be removed, but everything else looked good to me

There appears to be a 1 second sleep in the code at startup that should be removed, but everything else looked good to me

There will not be a sleep because we've added the wakeup event of EVFILT_USER before start polling. Unless you're talking about those platforms where EVFILT_USERs are broken?

Yes, that would be still be a regression, even if it isn't everywhere. Although I believe those platforms should fail to add the event which would immediately error also. There just does not appear to be a reason for this to need a sleep, since the EV_ADD alone should either succeed or fail immediately

Yes, that would be still be a regression, even if it isn't everywhere. Although I believe those platforms should fail to add the event which would immediately error also. There just does not appear to be a reason for this to need a sleep, since the EV_ADD alone should either succeed or fail immediately

Okay then,although I'm not certain if the kevent call will return immediately with the wakeup event that was added before the polling.

I'll verify it later and get back to you.

If you pass 0 for changes, the documentation says it is guaranteed to return immediately either with success or EINVAL that the filter type is invalid to add

If you pass 0 for changes, the documentation says it is guaranteed to return immediately either with success or EINVAL that the filter type is invalid to add

I've verified it and it did return in no time carrying the wakeup event. This sentence in the man pages should endorse it:

To effect a poll, the timeout argument should be non-NULL, pointing to a zero-valued timespec structure.

Perfect! That disturbing failure of the flaky test disappeared this time, I think we're good to go!

Cheers! 🥂