roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 211 forks source link

Configurable limits #610

Open gavv opened 11 months ago

gavv commented 11 months ago

This issue tracks progress of implementing various limits.

option default value description notes
:white_large_square: --max-mem 0 (no limit) max memory used by context in total
:white_large_square: --max-sessions 0 (no limit) max active sessions
:white_large_square: --max-session-mem 0 (no limit) max memory used by one session (including packets and frames)
:white_large_square: --max-sockets 0 (no limit) max active sockets
:white_large_square: --max-socket-mem 0 (no limit) max memory used by queued and pre-buffered packets belonging to socket and not routed to session yet
:white_large_square: --max-packet-size roc-send: deduce from --packet-len, roc-recv: 2K max bytes per packet rename from --packet-limit, rework default value
:white_large_square: --max-frame-size roc-send, roc-recv: deduce from --frame-len max bytes per frame rename from --frame-limit, rework default value

Steps:

gavv commented 11 months ago

Things to be covered (directly or indirectly) by defined limits:

nolan-veed commented 11 months ago

Shall I tackle this one?

gavv commented 11 months ago

Hi, you're welcome!

I guess we'd need to discuss implementation, I didn't try to go into great detail here and I suppose implementing memory limits would be not so trivial. I'll post my thoughts here.

Meanwhile, feel free to start with this part (it's anyway should be dome first I think):

Related changes: rename --packet-length to --packet-len, --packet-limit to --max-packet-size (roc-send) rename --frame-length to --frame-len, --frame-limit to --max-frame-size (roc-send, roc-recv) add parse_size() and used it for --max-packet-size and --max-frame-size

I was planning to open a separate issue for parse_size() and already prepared a text:

Implement parse_size() function, which parses bytes sizes.

For input strings like "1K", "1M", etc, it should return 1024, 1048576, etc.

We have similar function parse_duration() that parses strings like "1ms", "1s".
We can place both functions in same header.

Also add tests.

After adding new function, use it for --max-packet-size and --max-frame-size in
roc-send and roc-recv, similar to how parse_duration() is used for
--packet-len and --frame-len.

Also update help message and manual pages correspondingly.
gavv commented 10 months ago

I've updated the issue description and simplified the set of limits a bit.

Here are some thoughts regarding implementation.

As mentioned in the issue, we want to limit memory per-socket, per-session, and globally - so far it's the simplest yet reliable way I can think of. These three limits will cover the whole flow:

                   covered by --max-mem
                             /\
 ---------------------------------------------------------
|                                                         |

socket => network queue => prebuf queue => session pipeline

|                                     |    |              |
 --------------------------------------     --------------
                  \/                              \/
   covered by --max-socket-mem            covered by --max-session-mem

Generally, roc_netio allocates packet and adds it to network queue. Then roc_pipeline fetches it and routes it to session, or maybe moves it to prebuf queue and routes to session later (see #609).

Please also take a look at this two pages:

So I came up with the following limits:

Here is how the user may use this limits:

Depending on use case, the user may want to set only some of the limits, or all of them for most fine-grained control.


Besides packets, session pipeline allocates other things: pipeline elements, sample buffers, and other temporary buffers (e.g. for FEC).

However, in the end, all allocations are done either via IArena or via IPool, and they are very good candidates where we can implement limits.


Since there are multiple consumers that are subject to the same limit, it feel natural that we need to develop a limiter that is independent of consumer, that will be shared by pools, arenas, etc.

We currently have core::RateLimiter. By analogy, we can implement core::MemoryLimiter. It can have methods like bool acquire(num_bytes) and release(num_bytes). It won't manage memory itself, just keep track of hom much memory is already acquired and whether the limit is reached.

Then we could implement decorators for arena and pool: LimitedArena and LimitedPool. LimitedArena would have a reference to inner IArena + a reference to MemoryLimiter. Before making an allocation, it would ask limiter whether it can acquire more memory. And in deallocation it will tell limiter that memory is releases. Same for LimitedPool.

This will significantly simplify memory limiting for sessions. Currently all sessions share same arena & pool. Now we just need to pass to each session its own instance of LimitedArena and LimitedPool, like this:

              session
            /         \
   LimitedArena      LimitedPool
     |      \         /      |
     |     MemoryLimiter     |
     |     (per-session)     |
     |                       |
   HeapArena                 Pool
(shared between sessions)   (shared between sessions)

We can also extend this approach to add global limit too:

              session
            /         \
   LimitedArena      LimitedPool
     |      \         /      |
     |     MemoryLimiter     |
     |     (per-session)     |
     |                       |
   LimitedArena       LimitedPool
     |      \         /      |
     |     MemoryLimiter     |
     |       (global)        |
     |                       |
   HeapArena                 Pool
   (global)                (global)

The same approach can be used for sockets: we can create per-socket LimitedPool wrapper for shared pool.


There is one more thing that we need to handle.

When packet is routed from network/prebuf queue to a session, it actually becomes subject of other limit.

Before routing, packet should be covered by per-socker limit, and after routing, it should be covered by per-session limit.

To ahieve this, we can store a reference to its owning MemoryLimiter in the packet. When roc_piepline routes packet to a session, it should release packet from its original per-socket limiter and acquire it in its new per-session limiter.

We could add a method like bool Packet::transfer_to(MemoryLimiter*) that will perform this operation.


It should be easy to split implementation into multiple steps/PRs. E.g. first add MemoryLimiter + tests, then LimitedArena, then LimitedPool, then implement per-session limits, then per-socket, then global.

And before all this, it would be nice to implement "refactor current options" section first, so that new options will be consisten with existing ones.

In the end we'll also need to expose limits in C API.


Let me know what do you think about all this!

gavv commented 10 months ago

BTW, if it feels a bit too big, feel free to work on only part of the issue - the issue is not urgent and really any help is appreciated here.

gavv commented 10 months ago

One more change that we'll need to do to make this approach work.

We have BufferFactory and PacketFactory classes, that are convenient wrappers for Pool<Buffer> and Pool<Packet>. Currently we create factories in node::Context and then pass them to all components in roc_pipeline.

Since now roc_pipeline will need access to pools (to wrap them with LimitedPool adapter), we should change it:

nolan-veed commented 10 months ago

Thanks for the awesome detail. Appreciate it. I'll go with your suggestions. I haven't gone into enough codebase areas to provide any opinions on the approach, but will let you know, if anything comes up.

I'll start on this probably tomorrow. I think 1 PR for the initial renaming work is the first thing. I'll then see if I can break up the remaining big chunk of work into separate PRs.

gavv commented 10 months ago

One more small thought: since now we'll have more than one implementation of IPool (Pool and LimitedPool), it makes sense to rename Pool to something more specific, say, SlabPool. I've updated the task.

nolan-veed commented 10 months ago

It should be easy to split implementation into multiple steps/PRs. E.g. first add MemoryLimiter + tests, then LimitedArena, then LimitedPool, then implement per-session limits, then per-socket, then global.

I will start work on this. I'll create a PR for the classes first (with tests) and then another one to integrate them into system.

gavv commented 10 months ago

Great!

I think "factories" part from task description may be extracted into separate PR too.

nolan-veed commented 9 months ago

I think "factories" part from task description may be extracted into separate PR too.

Starting on this.