roc-streaming / roc-toolkit

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

Extract ListImpl class from List class #598

Closed gavv closed 1 month ago

gavv commented 9 months ago

core::List<T> template implements intrusive doubly-linked list.

To reduce code size and compilation times, it would be nice to extract its internals to a non-template implementation class. All existing code will continue using List<T>, however its methods will mostly just invoke similar methods of ListImpl and make some type casts. This way, the template part will be small, and non-template part will be compiled once and reused.

We did a very similar job for core::MpscQueue here:

rjwignar commented 9 months ago

I might need a couple days to study the changes in the linked issue and PR but I think I can do this. I would like to try it out.

gavv commented 9 months ago

You're welcome, thanks!

rjwignar commented 8 months ago

Apologies for the delay. I've been preoccupied with school and other commitments. Wanted to give an update on my progress.

So far I've only been able to extract the head_ and size_ data members (along with the List() no-argument constructor) to the list_impl module.

I'll need some more time (and perhaps some guidance) to extract the remaining methods :

but I'd like to keep working on this.

gavv commented 6 months ago

@rjwignar Hi, do you still have plans on this?

rjwignar commented 5 months ago

@gavv Apologies for the radio silence. I couldn't figure out how to extract the remaining methods, even after referring to #593 . Please unassign me.

I am however, interested in seeing how this would be done.

veronikaro commented 4 months ago

@gavv is this issue still relevant? If so, I would like to work on it if you do not mind since I am already got familiar with the class itself. Thanks!

gavv commented 4 months ago

@veronikaro Absolutely, thanks!

rjwignar commented 4 months ago

@veronikaro Thank you for taking this on! I'm excited to learn how you refactor this.

gavv commented 1 month ago

Merged via bef7210761ad2ff063da7ce55e7a974a3dc9c6fd