Improve buffer allocation for RTP Packets

[ibc]

In RTP/SharedPacket.cpp we do:

/* Static. */

// When cloning a RTP packet, a buffer is allocated for it and its length is
// the length of the Packet plus this value (in bytes).
static constexpr size_t PacketBufferLengthIncrement{ 100 };
// Callback to pass to every cloned RTP Packet to deallocate its buffer once
// the Packet releases its buffer (for example when the Packet is destroyed).
static thread_local Serializable::BufferReleasedListener PacketBufferReleasedListener =
  [](const Serializable* packet, uint8_t* buffer)
{
	delete[] buffer;

#ifdef MS_DUMP_RTP_SHARED_PACKET_MEMORY_USAGE
	SharedPacket::allocatedMemory -= packet->GetBufferLength();

	MS_DUMP_CLEAN(
	  0,
	  "[RTC::RTP::SharedPacket] memory deallocated [packet buffer:%zu, total allocated memory:%" PRIu64
	  "]",
	  packet->GetBufferLength(),
	  SharedPacket::allocatedMemory);
#endif
};

void SharedPacket::StorePacket(RTC::RTP::Packet* packet)
{
	MS_TRACE();

	const size_t bufferLength = packet->GetLength() + PacketBufferLengthIncrement;
	auto* buffer              = new uint8_t[bufferLength];
	auto* clonedPacket        = packet->Clone(buffer, bufferLength);

	// Set a listener in the Packet to deallocate its buffer once the Packet
	// is destroyed or releases its internal buffer.
	clonedPacket->SetBufferReleasedListener(std::addressof(PacketBufferReleasedListener));

	this->sharedPtr->reset(clonedPacket);

#ifdef MS_DUMP_RTP_SHARED_PACKET_MEMORY_USAGE
	SharedPacket::allocatedMemory += bufferLength;

	MS_DUMP_CLEAN(
	  0,
	  "[RTC::RTP::SharedPacket] memory allocated [packet buffer:%zu, total allocated memory:%" PRIu64
	  "]",
	  clonedPacket->GetBufferLength(),
	  SharedPacket::allocatedMemory);
#endif
}

This means that for each received RTP packet we are allocating a buffer of bufferLength = packet->GetLength() + 100 bytes.

While nothing is wrong here, we could make it better by having a pre-allocated big buffer in which we could use free regions of it to clone packets. It would be more efficient than calling new uint8_t[bufferLength] for each packet.

Of course, any allocator mechanism consists on having chunks of fixed size (aligned to 8 bytes for efficiency). This means that we should decide a max valid length of RTP packet and should discard those received packets that are bigger than that, but that shouldn't be a problem.

Existing stuff

• https://faraz.work/awesome-cpp/#memoryallocation
• https://github.com/mattconte/tlsf

[jmillan]

We have a long time PR which allows this. It uses an internal vector of buffers.

When a buffer is requested for usage:

• If the length of the vector is > 0 then it returns the last element and performs a pop_back(), removing it from the vector.
• If the length of the vector is = 0 it allocates a buffer and returns it.

When a buffer is returned back from usage:

• The buffer is put back on the vector.

This approach is very dynamic and simple. There will be as many allocations as the maximum number of in-use buffers at a certain time. This is, if within the live of the server there was a peak with 1000 buffers in use, only 1000 memory allocations are required for the whole lifetime of the server.

Talking about performance, take a look at the description of the PR where there are comparative numbers.

we could make it better by having a pre-allocated big buffer in which we could use free regions of it to clone packets

This does not necessarily need to better. Allocating big memory regions may be more costly that allocating a bigger number of smaller ones because of surpassing the page size. Also this approach would increase complexity as the code would now need to manage the regions in use.

The conclusions on the mentioned PR was that with other injectable allocators such as jemalloc and mimalloc which perform better than malloc, and use arenas, etc for this kind of optimisations, it was not that evident that having such a pool was a benefit considering the added complexity.

[ibc]

My idea was more like this:

• Have a MemoryAllocator that initially allocates a buffer of (for example) 2 MB.
• That buffer is split into slot of 1500 bytes (let's imagine that's our max allowed RTP packet length).
• SharedPacket requests a slot to the MemoryAllocator so store a RTP packet and receives some numeric idx as return value (which must later be used to tell the MemoryAllocator that the packet buffer can be reused.
• If eventually there are no more available slots available, then MemoryAllocator internally allocates another buffer of 2 MB and stores the RTP packet in there, and so on.

Have you took a look to all the existing options in https://faraz.work/awesome-cpp/#memoryallocation that basically do something like this?

[jmillan]

My idea was more like this:

That approach is what I say that adds more complexity as it allocates a big chunk of memory and needs to internally keep track of in-use/freed ones. IMO it does not necessarily beat the approach of the mentioned PR.

Have you took a look to all the existing options in https://faraz.work/awesome-cpp/#memoryallocation that basically do something like this?

Basically it points mimalloc, jemalloc, snmalloc etc. Those are allocators that can already be used with mediasoup, as I commented in my previous comment:

The conclusions on the mentioned PR was that with other injectable allocators such as jemalloc and mimalloc which perform better than malloc, and use arenas, etc for this kind of optimisations, it was not that evident that having such a pool was a benefit considering the added complexity.

[ibc]

I didn't mean mimalloc, jeamalloc or any other of those things that basically override malloc() and free(). I meant pure C/C++ libraries such as https://github.com/foonathan/memory.

Obviously I have no experience on this topic and you already experimented these things. However my understanding is that those mimalloc, jeamalloc "addons" are the only alternative when the program itself doesn't manage its memory properly.