Linux specific IPC RingBuff using EventFD for async reader wakeup #10

Open
guille wants to merge 334 commits from one_ring_to_rule_them_all into shm_apis
Collaborator
  • TODO @guille, write a description of this IPC transport feat!

@guille’s re-org list

  • move EventFdIO stuff (and related util fns) into a separate module, maybe just ._eventfd

  • not sure where we want the RingBuffReceiver/Sender interfaces put yet?

    • maybe these should go into a tractor._buffer or ._buf mod for now and we’ll eventually likely move them under the tractor.ipc subpkg (see my list below).

@goodboy’s surrounding re-org needed around this!

  • we should make a new tractor.ipc subpkg with an __init__.py that only exports all the currently used lower level bits from the current tractor._ipc

  • the set of cmds to do this should be trying to preserve as much history in module-files as possible,

    • mkdir tractor/ipc

    • touch tractor/ipc/__init__.py

      • do import Channel as Channel for all refs like Channel that are used in other parts of the code base and obvi change those imports to now src from tractor.ipc instead of ._ipc ;)
    • git mv tractor/_ipc.py tractor/ipc/_chan.py

      • preserves the full history of the ._ipc mod.
      • we’ll (eventually) need to break up the mod content eventually, where we
        • leave all Channel related code in that new tractor.ipc._chan

        • move all the even lower-level transport code to a new .ipc._tcp and possibly a new .ipc._transport for the interface/types that other tranports (“backends”) can implement.

    • git mv tractor/_shm.py tractor/ipc/_shm.py

      • again, maintain history of the file as much as possible, so the above OP should be done, THEN IMMEDIATELY committed.
      • move out all the new stuff from @guille to a new module in tractor.ipc maybe .ipc.ringbuf or similar; and/or we might want to stick all the EventFdIO stuff to it’s own mod as well since it shouldn’t be directly tied to the ShmList/Array stuff right?
      • not sure where we want the RingBuffReceiver/Sender interfaces put yet (see above)
- [ ] TODO @guille, write a description of this IPC transport feat! --- #### @guille's re-org list - [x] move `EventFdIO` stuff (and related util fns) into a separate module, maybe just `._eventfd` - [x] not sure where we want the `RingBuffReceiver/Sender` interfaces put yet? - maybe these should go into a `tractor._buffer` or `._buf` mod for now and we'll eventually likely move them under the `tractor.ipc` subpkg (see my list below). --- #### @goodboy's surrounding re-org needed around this! - [x] we should make a new `tractor.ipc` subpkg with an `__init__.py` that **only** exports all the currently used lower level bits from the current `tractor._ipc` - [x] the set of cmds to do this should be trying to preserve as much history in module-files as possible, - `mkdir tractor/ipc` - `touch tractor/ipc/__init__.py` - [x] do `import Channel as Channel` for all refs like `Channel` that are used in other parts of the code base and obvi change those imports to now src from `tractor.ipc` instead of `._ipc` ;) - `git mv tractor/_ipc.py tractor/ipc/_chan.py` - preserves the full history of the `._ipc` mod. - we'll (eventually) need to break up the mod content eventually, where we - [x] **leave** all `Channel` related code in that new `tractor.ipc._chan` - [ ] **move** all the even lower-level transport code to a new `.ipc._tcp` and possibly a new `.ipc._transport` for the interface/types that other tranports ("backends") can implement. - `git mv tractor/_shm.py tractor/ipc/_shm.py` - [x] again, maintain history of the file as much as possible, so the above OP should be done, THEN IMMEDIATELY committed. - [x] move out all the new stuff from @guille to a new module in `tractor.ipc` maybe `.ipc.ringbuf` or similar; and/or we might want to stick all the `EventFdIO` stuff to it's own mod as well since it shouldn't be directly tied to the `ShmList/Array` stuff right? - [ ] not sure where we want the `RingBuffReceiver/Sender` interfaces put yet (**see above**)
guille added 1 commit 2025-03-12 19:16:32 +00:00
goodboy reviewed 2025-03-13 17:22:45 +00:00
default.nix Outdated
@ -0,0 +1,18 @@
{ pkgs ? import <nixpkgs> {} }:

Hmm should this be part of the repo though?

It’d be nice if we could also look into wtv they doin in nix land these days to integrate with uv 😉

Hmm should this be part of the repo though? It'd be nice if we could also look into wtv they doin in `nix` land these days to integrate with `uv` 😉
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 17:23:17 +00:00
@ -11,6 +14,7 @@ import tractor
from tractor._shm import (
open_shm_list,
attach_shm_list,
EventFD, open_ringbuffer_sender, open_ringbuffer_receiver,

keep the surrounding multi-line import style porfa!

keep the surrounding multi-line import style porfa!
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 17:24:00 +00:00
tractor/_shm.py Outdated
@ -33,3 +34,3 @@
)
from msgspec import Struct
from msgspec import Struct, to_builtins

again, please always use multi-line tuple style imports 🙏🙏

again, please always use multi-line tuple style imports 🙏:pray:
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 17:25:18 +00:00
tractor/_shm.py Outdated
@ -833,1 +834,4 @@
)
if platform.system() == 'Linux':

prolly moving this into a separate mod would be good.

as mentioned that should be easier once we re-org to a tractor.ipc subpkg 😉

prolly moving this into a separate mod would be good. as mentioned that should be easier once we re-org to a `tractor.ipc` subpkg 😉
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 17:27:38 +00:00
tractor/_shm.py Outdated
@ -834,0 +1030,4 @@
return self
async def __aexit__(self, exc_type, exc_value, traceback):
await self.aclose()

not following this, isn’t the whole point of being a trio.abc.SendStream to not have to implement these dunder meths?

Also, if .aclose() does nothing then seems even more strange that you’d call it ?

not following this, isn't the whole point of being a `trio.abc.SendStream` to **not have to** implement these dunder meths? Also, if `.aclose()` does nothing then seems even more strange that you'd call it ?
Poster
Collaborator

On latest commit we do have to do eventfd cleanup calls on aclose

but yeah removed __aexit__, super class impl calls aclose underneath

On latest commit we do have to do eventfd cleanup calls on `aclose` but yeah removed `__aexit__`, super class impl calls `aclose` underneath
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 17:28:43 +00:00
@ -141,6 +141,7 @@ class ActorNursery:
# a `._ria_nursery` since the dependent APIs have been
# removed!
nursery: trio.Nursery|None = None,
proc_kwargs: dict[str, any] = {}

luv this!

it might make even more sense if we eventually use a msgspec.Struct for this as well so we can explicitly decide what to allow passing through?

luv this! it might make even more sense if we eventually use a `msgspec.Struct` for this as well so we can explicitly decide what to allow passing through?
goodboy reviewed 2025-03-13 17:30:48 +00:00
tractor/_shm.py Outdated
@ -834,0 +1123,4 @@
create=False
)
async with RingBuffReceiver(
shm, EventFD(fd=write_event_fd), EventFD(fd=wrap_event_fd, omode='w'), start_ptr=start_ptr

generally speaking (as i’ve kinda implied above) try to keep a clean "multi-line inputs & outputs* style to code.

so here that would be,

async with RingBuffReceiver(
    shm, 
    EventFD(fd=write_event_fd),
    EventFD(fd=wrap_event_fd, omode='w'),
    start_ptr=start_ptr,
) as r:
    yield r

could could go further and do it with the EventFD inputs as well ;)

the reason(s) i prefer this,

  • easier to edit the code when you need to change inputs, outputs, imports etc..
  • we don’t really care about LoC length.. it is py after all Bp
  • IMHO it’s alot easier to read instead of long lines with no std way that details per scope are broken up
generally speaking (as i've kinda implied above) try to keep a clean "multi-line inputs & outputs* style to code. so here that would be, ```python async with RingBuffReceiver( shm, EventFD(fd=write_event_fd), EventFD(fd=wrap_event_fd, omode='w'), start_ptr=start_ptr, ) as r: yield r ``` could could go further and do it with the `EventFD` inputs as well ;) the reason(s) i prefer this, - easier to edit the code when you need to change inputs, outputs, imports etc.. - we don't really care about LoC length.. it is py after all Bp - IMHO it's **alot easier to read** instead of long lines with no std way that details per scope are broken up
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 18:16:41 +00:00
tractor/_shm.py Outdated
@ -834,0 +914,4 @@
def close_eventfd(fd: int) -> int:
'''
Close the eventfd.
'''

Style i’ve been rolling with for doc-strings is to have a blank newline before the last ''' 🙏

Style i've been rolling with for doc-strings is to have a blank newline before the last `'''` 🙏
guille marked this conversation as resolved
goodboy reviewed 2025-03-13 18:17:31 +00:00
tractor/_shm.py Outdated
@ -834,0 +920,4 @@
raise OSError(errno.errorcode[ffi.errno], 'close failed')
class EventFD:

yeah, i almost want to say you can move this stuff already into a new module?

yeah, i almost want to say you can move this stuff already into a new module?
guille marked this conversation as resolved
guille added 1 commit 2025-03-13 23:18:17 +00:00
ab1a60bc97
General improvements
EventFD class now expects the fd to already be init with open_eventfd
RingBuff Sender and Receiver fully manage SharedMemory and EventFD lifecycles, no aditional ctx mngrs needed
Separate ring buf tests into its own test bed
Add parametrization to test and cancellation
Add docstrings
Add simple testing data gen module .samples
guille added 1 commit 2025-03-13 23:41:37 +00:00
guille added 1 commit 2025-03-13 23:59:20 +00:00
guille force-pushed one_ring_to_rule_them_all from 07dab8519e to 41e84cc701 2025-03-14 00:02:23 +00:00 Compare
guille added 1 commit 2025-03-14 00:10:37 +00:00
guille added 1 commit 2025-03-14 00:15:26 +00:00
guille added 1 commit 2025-03-14 00:25:55 +00:00
guille added 2 commits 2025-03-14 01:47:54 +00:00
guille added 1 commit 2025-03-14 02:12:28 +00:00
guille force-pushed one_ring_to_rule_them_all from 3127db8502 to dd17aa4205 2025-03-14 02:41:59 +00:00 Compare
guille added 1 commit 2025-03-14 03:26:39 +00:00
guille added 1 commit 2025-03-16 16:54:47 +00:00
guille force-pushed one_ring_to_rule_them_all from 3db500bd2b to 3c5420f4c9 2025-03-16 17:14:38 +00:00 Compare
guille added 2 commits 2025-03-17 03:02:41 +00:00
d6721f06df
Better encapsulate RingBuff ctx managment methods and support non ipc usage
Add trio.StrictFIFOLock on sender.send_all
Support max_bytes argument on receive_some, keep track of write_ptr on receiver
Add max_bytes receive test test_ringbuf_max_bytes
Add docstrings to all ringbuf tests
Remove EFD_NONBLOCK support, not necesary anymore since we can use abandon_on_cancel=True on trio.to_thread.run_sync
Close eventfd's after usage on open_ringbuf
5cec4ee943
Switch `tractor.ipc.MsgTransport.stream` type to `trio.abc.Stream`
Add EOF signaling mechanism
Support proper `receive_some` end of stream semantics
Add StapledStream non-ipc test
Create MsgpackRBStream similar to MsgpackTCPStream for buffered whole-msg reads
Add EventFD.read cancellation on EventFD.close mechanism using cancel scope
Add test for eventfd cancellation
Improve and add docstrings
guille added 1 commit 2025-03-18 16:19:55 +00:00
guille added 1 commit 2025-03-18 16:47:47 +00:00
guille added 1 commit 2025-03-19 01:49:54 +00:00
1bbf1f7ab5
Add direct read method on EventFD
Type hint all ctx managers in _ringbuf.py
Remove unnecesary send lock on ring chan sender
Handle EOF on ring chan receiver
Rename ringbuf tests to make it less redundant
goodboy reviewed 2025-03-19 14:22:38 +00:00
@ -0,0 +216,4 @@
destaddr = destaddr or self._destaddr
assert isinstance(destaddr, tuple)
stream = await trio.open_tcp_stream(

Heh, i forgot this was hard coded at the Channel level..

We should obvi move this down to MsgpackTCPStream.connect() (or something) and then make it easy to just swap to USD via the open_unix_stream() call?

https://github.com/python-trio/trio/blob/main/src/trio/_tests/test_highlevel_open_unix_stream.py#L65

which if you look at the factory, also returns a SocketStream B)

https://github.com/python-trio/trio/blob/main/src/trio/_highlevel_open_unix_stream.py#L38

Heh, i forgot this was hard coded at the `Channel` level.. We should obvi move this down to `MsgpackTCPStream.connect()` (or something) and then make it easy to just swap to USD via the `open_unix_stream()` call? https://github.com/python-trio/trio/blob/main/src/trio/_tests/test_highlevel_open_unix_stream.py#L65 which if you look at the factory, also returns a `SocketStream` B) https://github.com/python-trio/trio/blob/main/src/trio/_highlevel_open_unix_stream.py#L38

If the typing is the same in trio it might even be worth changing to a MsgpackSocketStream ??

and then we just let the UDS vs. TCP be a(n introspect-able) impl deat?

If the typing is the same in `trio` it might even be worth changing to a `MsgpackSocketStream` ?? and then we just let the UDS vs. TCP be a(n introspect-able) impl deat?

stream of thoughts here (punzone), but the only thing that’s obvi going to be incompat is the address typing,

for UDS it’s a file path and for TCP it’s obvi an ipv4 socket-addr..

so maybe we need to wrap addrs in an interface that can be called to deliver the appropriate type/format to the respective transport factories,

tuple[str, int] => open_tcp_stream()

str => open_unix_socket()

??

stream of thoughts here (punzone), but the only thing that's obvi going to be incompat is the address typing, for UDS it's a file path and for TCP it's obvi an ipv4 socket-addr.. so maybe we need to wrap addrs in an interface that can be called to deliver the appropriate type/format to the respective transport factories, `tuple[str, int]` => `open_tcp_stream()` `str` => `open_unix_socket()` ??
goodboy requested review from goodboy 2025-03-19 18:57:31 +00:00
goodboy requested review from zoltan 2025-03-19 18:57:38 +00:00
guille added 1 commit 2025-03-21 00:12:18 +00:00
guille force-pushed one_ring_to_rule_them_all from 6e113d2150 to 0208a4728f 2025-03-22 19:40:49 +00:00 Compare
guille changed title from Linux specific IPC RingBuff using EventFD for async reader wakeup to Linux specific IPC RingBuff using EventFD for async reader wakeup 2025-03-22 19:41:08 +00:00
guille changed target branch from uv_migration to shm_apis 2025-03-22 19:41:08 +00:00
guille added 1 commit 2025-03-22 19:54:13 +00:00
This pull request has changes conflicting with the target branch.
  • examples/debugging/restore_builtin_breakpoint.py
  • pyproject.toml
  • requirements-test.txt
  • setup.py
  • tests/devx/test_debugger.py
  • tests/devx/test_tooling.py
  • tests/test_shm.py
  • tractor/_context.py
  • tractor/_ipc.py
  • tractor/_rpc.py
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b one_ring_to_rule_them_all shm_apis
git pull origin one_ring_to_rule_them_all

Step 2:

Merge the changes and update on Gitea.
git checkout shm_apis
git merge --no-ff one_ring_to_rule_them_all
git push origin shm_apis
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: goodboy/tractor#10
There is no content yet.