untagged bindings to epoll_ctl

[craff]

This is usefull when you frequently change the epoll list (which is not the case in general if you use ET and non blocking socket, because you can wait for both in and out all the time and only have to setup the epoll control when initializing the socket).

There is one problem: we know on linux that file_descr are int ... but to untag it we need a dirty Obj.magic.

[lindig]

The values that are being passed are all primitive:

• EPoll file descriptor: int
• Unix.file_descriptor: int
• Events.t: int

Hence, there is no allocation overhead at the boundary of this function call. The values are passed in registers or on the stack, not on the heap. I am therefore doubtful that anything is gained here but you seem to think otherwise. Maybe it's worth looking at some intermediate code.

[craff]

You are probably right for noalloc being useless. The compiler might miss the optimization because file_descr are not int on all windows and is not classified as immediate by the compiler. I will check the generated code. Still untagged is useful, it allows optimization on the ocaml side when tagging/untagging. Le 5 août 2023 03:57:03 GMT-10:00, Christian Lindig ***@***.***> a écrit :

…

The values that are being passed are all primitive: * EPoll file descriptor: int * Unix.file_descriptor: int * Events.t: int Hence, there is no allocation overhead at the boundary of this function call. The values are passed in registers or on the stack, not on the heap. I am therefore doubtful that anything is gained here but you seem to think otherwise. Maybe it's worth looking at some intermediate code. -- Reply to this email directly or view it on GitHub: #7 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

[edwintorok]

Might be useful to separate the 'untagged' and the 'noalloc' changes. The 'noalloc' changes might be useful on their own and don't require any Obj.magic.

And if the values are already integers then the benefit of 'untagged' would be that you may be able to call the 'libc' function directly without an extra C stub, but the functions that benefit from aren't in a fastpath, the one function that may be called often (the actual poll/wait functions) cannot be 'noalloc' because they invoke callbacks into OCaml.
I'd suggest to measure what performance benefit using untagged would yield here, and whether there are options other than Obj.magic to achieve that.

[craff]

I wait that we converge on PR #9 and after that I will do benchmark. I let this PR open until I create two new one, if it is worth it.

[lindig]

In general, I think the performance of the wait call is the one to watch. This call allocates space on the stack based on the maximum number of file descriptors to report. I wonder whether this should have a safety check to make sure the number is between 1 and some upper limit. The upper limit does not limit the total number of FDs that can be watched; only the number of FDs that are reported back per call. Or we can leave that to the system call to check as it is now.

[edwintorok]

I haven't checked in detail, but limiting the number of fds reported back by epoll_wait can potentially lead to starvation issues, e.g. if you have M FDs that are ready, but ask epoll to tell you only about N, you process those N, and then you ask epoll_wait again. Assuming that those N will have more data available by the next time you call epoll_wait will the syscall now give you the same first N FDs back, or will it give you some of the other FDs?
If it always gives you the first N then on a sufficiently fast network you may end processing only those N sockets, and never looking at the rest M-N sockets, i.e. a starvation issue.

In theory a (root) process can raise its hard limits to have 1073741816 fds, which definitely won't fit on the stack:

 cat /proc/sys/fs/nr_open 
1073741816

I don't have a use-case for that many file descriptors at the moment, but rather than limiting the number of fds watched (which can lead to starvation issues, and depend on things like the available stack size which depends on how deep you are in a recursive call/etc.) we could perhaps preallocate the 'events' struct in Polly.t (i.e. instead of having Polly.t contain just an fd)

[lindig]

The number of FDs reported can vary from call to call. The space is only required while iterating over them. Hence I thought it clever to allocate this space on the stack and not where it would have to be garbage collected. Given that the number is not fixed, I don't see how allocating it somewhere else is better.

[edwintorok]

Perhaps a unit test would be useful that uses an arbitrarily small limit (at an extreme just '1') for the number of events to iterate over while continuously generating data on N socketpairs (e.g in another process forked from the main unit test), and check for starvation issues.
If there are no starvation issues then we could have a default limit of events to avoid stack overflows in the general case.

I checked the manpage again and it claims it will round-robin: https://man7.org/linux/man-pages/man2/epoll_wait.2.html#NOTES, and assuming we can rely on the kernel implementation here to not have bugs then we can keep using alloca (to be tested with the unit test, possibly on multiple kernel versions, and the test would be useful to have to detect future kernel regressions here, I expect this to be a corner case that is not well tested in general).

[craff]

I should have spotted this before, there is a bit potential gain there.

I think we should change the interface, the current complexity is O(val_max), should be O(reported_fd). This is easy to solve, replace the val_max parameter by a preallocated custom block or bytes. This would allow to use large val_max to avoid starvation issues. I am doing a PR, and will come back on noalloc/untagged later.

[craff]

I replaced this by PR #12. Might work on untagged later, but the conversion between Unix.file_descr and int is necessary to be clean.