Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only 32 timerfds are possible in the current implementation #8

Closed
myfreeweb opened this issue Jun 18, 2019 · 6 comments
Closed

Only 32 timerfds are possible in the current implementation #8

myfreeweb opened this issue Jun 18, 2019 · 6 comments
Assignees

Comments

@myfreeweb
Copy link

@myfreeweb myfreeweb commented Jun 18, 2019

I was wondering why I couldn't launch new Wayland clients (the compositor would return the "no memory" error) when working very actively on stuff.. and DTrace led me to timerfd creation failing.

Looks like this timerfd implementation creates one POSIX timer and one thread per timer (btw, threads should be named, I just realized where all the threads in my compositor came from.. also looks like one thread should be enough, signal info includes a unique ID per timer).

And.. turns out you can only create TIMER_MAX timers, which is defined as 32 in sys/timers.h :D

I'm not sure what's the right solution. Looks like timer limit should be increased in the kernel since the POSIX timer limit on Linux is relatively huge. I just created FreeBSD bug 238675 for this.

But maybe there should be a fallback to userspace thread based timing?

@jiixyj

This comment has been minimized.

Copy link
Owner

@jiixyj jiixyj commented Jun 18, 2019

It should be possible to always use kqueue EVFILT_TIMER to implement CLOCK_MONOTONIC timerfd timers. Since Wayland only uses CLOCK_MONOTONIC timers this should solve the problem.

I guess in that case we even won't need to spawn a thread. Just use the kqueue for readiness notifications and have a special read that implements timerfd read semantics. The only thing that stays problematic is different times in it_value and it_interval since EVFILT_TIMER does not provide an interface for this. But nobody uses this anyway, so...

@Crest

This comment has been minimized.

Copy link

@Crest Crest commented Jun 18, 2019

It would probably be better to implement a priority heap and use just a single timer, because kern.kq_calloutmax isn't all that high either with a default of just 4096.

@jiixyj jiixyj self-assigned this Jun 21, 2019
@jiixyj

This comment has been minimized.

Copy link
Owner

@jiixyj jiixyj commented Jun 22, 2019

I've now implemented a 'fast path' using kqueue EVFILT_TIMER. The old POSIX timer based solution is a fallback for the following cases:

  • CLOCK_REALTIME based timers
  • when specifying TFD_TIMER_ABSTIME
  • when it_value and it_interval are different

It should be possible to also use EVFILT_TIMERs for some of the above cases, but this isn't implemented currently.

While I didn't test with Wayland I've added some test cases. I'm curious if more than 32 clients are possible now.

@jiixyj

This comment has been minimized.

Copy link
Owner

@jiixyj jiixyj commented Jun 22, 2019

It would probably be better to implement a priority heap and use just a single timer, because kern.kq_calloutmax isn't all that high either with a default of just 4096.

This is a good idea! I wouldn't implement this prematurely, though. The current implementation has the advantage that it stays reasonably close to the kernel primitives. I think Wayland/libinput and others already implement something like a priority heap on top of timerfd.

But you're right, the kern.kq_calloutmax limit of 4096 is something to think about...

@myfreeweb

This comment has been minimized.

Copy link
Author

@myfreeweb myfreeweb commented Jun 23, 2019

Thanks a lot! I can confirm that new threads aren't being created in the compositor and I can launch more clients than I did before when I discovered the issue.

4096 is a much more reasonable limit (though I guess it's global?)

Wayland/libinput and others already implement something like a priority heap on top of timerfd

libwayland-server literally just creates a timerfd for each new connecting client and starts polling it..

@jiixyj

This comment has been minimized.

Copy link
Owner

@jiixyj jiixyj commented Nov 2, 2019

Just let me know if you find any issues with this.

@jiixyj jiixyj closed this Nov 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.