Reduce number of allocations

[algesten]

The impls has a few Vec<u8> arguments. It would be good to look through the instances of these to see where we can re-use buffers (or use ring buffers), and where we can use lifetimes to avoid internal allocation.

Zero allocations is a non-goal, but we can do better than we currently do.

[VictorBulba]

It would be nice to add support for rust allocators api. As this implementation works more like state machine, it could benefit a lot from bump allocators which could be reset each polling iteration

[algesten]

@VictorBulba Cool! I never used those, but sounds interesting!

[k0nserv]

I was thinking some more about this, for both async and sync APIs it's typically to use &[u8] for writing and &mut [u8] for reading. This way the caller is completely in charge of allocation decisions and the reference only lives for the duration of the IO call.

In sans-IO this is not as trivial. In the case of a write the caller needs to pass owned data so progress on writing can be made at a later stage. In str0m this manifests as Writer::write taking a impl Into<Vec<u8>> rather than a &[u8]. For read it similarly manifests as str0m returning Vec<u8>s instead of taking a &mut [u8] and writing into that.

The bump allocator idea is interesting, but it could only be used for temporary allocations during each handle_timeout and poll_output call. For persistent allocations(those returned out from str0m and those given to str0m as input) I'm not sure it would work as well.

[VictorBulba]

Str0m could use some circular buffer which is a contiguous chunk of memory, and allows append bytes to the end, and consuming then from the start.

Something like:

// BytesBuffer is a special circular buffer for str0m

let buf = BytesBuffer::with_capacity(1024);

// It can be written to
buf.write(&[0x01, 0x02, 0x03, 0x04, 0x05, 0x06]);

// Reading updates the circular buffer's offset,
// so consumed memory can be resused
{
    // Read guard keeps mutable reference to the buffer,
    // so it can't be written to while the guard is alive
    //
    // fn consumed_read<'a>(&'a mut self, len: usize) -> ReadGuard<'a>

    let read_guard = buf.consumed_read(3);

    assert_eq!(read_guard.get_bytes(), &[0x01, 0x02, 0x03]);

    drop(read_guard);
}

// Previous read memory has been consumed
assert_eq!(buf.consumed_read(2).get_bytes(), &[0x04, 0x05]);

str0m could use multiple of such buffers for different purposes. When user does write str0m could copy user bytes into that buffer, so it acts as a "queue". Also ReadGuard can be used as a return type for transmit operations, so it references str0m internal bytes chunk, eliminating all those Vec<u8> and Vec<Vec<u8>>

[thomaseizinger]

Parsing a StunMessage currently also allocates because all attributes are collected into a Vec. Given that we only support a constrained set of attributes, this can probably be improved upon by either keeping each attribute in an Option or using something like SmallVec.

Keeping things within an Option or a static array would have the advantage that we likely could make StunMessage Copy! Note that the use of Copy doesn't mean we will copy the payload. The payload is stored by reference so we are only copying references, not the actual data. SmallVec will allocate on the heap once the inline-storage is exceeded so it cannot be Copy.

[algesten]

Good point!

I favor Option. This is actually exactly equivalent to our RTP header extension values, where do already do it: https://github.com/algesten/str0m/blob/main/src/rtp/ext.rs#L846 – it would be really nice to make StunMessage the same.

[thomaseizinger]

Good point!

I favor Option. This is actually exactly equivalent to our RTP header extension values, where do already do it: main/src/rtp/ext.rs#L846 – it would be really nice to make StunMessage the same.

I submitted a PR for for that: #425