ObjectPoolAllocator

[jmillan]

This is an extract from a bigger "Memory Optimizations" PR #675

It contains only the ObjectPoolAllocator part with a few exceptions:

• RtpPacket::Parse() does return a RtpPacket instance. In Memory optimizations #675 it returned a shared pointer but since shared pointers are just needed for cloned packets (for retransmission), the former method does not need to create one.
• RtcpCompoundPacket is left as it was. This is, a new one is created when needed (this will be enhanced in a separate PR).

As per my tests I'm not seeing much difference with this and v3. There is maybe some noticeable CPU enhancement.

Please @nazar-pc , @Hartigan give it a try and instrument this if possible to see how it enhances comparing to v3. I'll do some more testing just to make sure this addition is worth it.

Performed performance unit tests:

Parsed RtpPackets	ObjectPoolAllocator (sec)	new/delete (sec)
10K	0.0028	0.0039
100K	0.027	0.038
1M	0.26	0.33

Cloned RtpPackets	ObjectPoolAllocator (sec)	new/delete (sec)
10K	0.0034	0.0049
100K	0.033	0.049
1M	0.33	0.48

ObjectPoolAllocator is definitely more performant as it reuses memory instead of freeing & re-allocating it.

[ibc]

Just a few things reviewed yet

[jmillan]

NOTE: Currently measuring the CPU time differences with pools VS new/delete.

[jmillan]

Converted to draft while re-testing re-profiling. Feel fee to try it out and review.

[nazar-pc]

@Hartigan please send multiple comments as a review together rather than individually, it will reduce number of GitHub notifications everyone receives.

[ibc]

Some cosmetic changes requested and a question. Other than that it looks very good however I'm not in a good shape to properly review these huge changes, but I trust you guys.

[ibc]

Which is the status of this PR?

[jmillan]

It's ready to merge. It's working properly. Even though we may want to enrich he pool allocator in the future is a good starting point.

[ibc]

Ok?

[ibc]

Reminder: this PR needs love.

[jmillan]

Yes, it's on my TODO

[jmillan]

I'll revive this PR for the new year.

[jmillan]

Guys, back to this topic.

I see the benefits of using an object allocator in favour of new() and delete(). On the other side I'm not 100% confortable with the given implementation here as we are also loosing track of used objects in case there is a memory leak anywhere.

My opinion here is keeping the branch for future consultation, and spend the proper time on a solution with which we feel more confortable when the need for this performance enhancement comes.

My personal experience here is that using a different allocator such us jemalloc increases substantially the performance out of the box. I know these are two different topics, but anyone could use an alternative allocator transparently, according to their needs.

What do you think @ibc, @nazar-pc?

[Hartigan]

Hi @jmillan

I agree, I have good experience with mimalloc for example. We may create separate MR only with smart pointers for memory safety

Thanks