rlogiacco / CircularBuffer

Arduino circular buffer library
GNU Lesser General Public License v3.0
312 stars 85 forks source link

Implement pass by reference for large objects #68

Closed boothinator closed 8 months ago

boothinator commented 2 years ago

Detect when the item type T is large and use pass-by-reference instead of pass-by-value for push() and unshift(). This should be faster for large objects, per the C++ Core Guidelines F.16. Additionally, provide implementations of pop(), shift(), first(), and last() that take a reference parameter for these large types.

This feature is implemented similarly to the IT parameter. The new template parameter TIn stores the input type for push() and unshift(). The TIn parameter defaults to the newly created Helper::Input::Type, which is T when sizeof(T) <= 2*sizeof(void *), and is const T & when sizeof(T) > 2*sizeof(void *). The other access methods are unchanged, although new versions are provided that take a reference to avoid having to return large objects, if desired by the caller.

rlogiacco commented 2 years ago

Thanks for the contribution. Would you please also add an Example and a section in the README describing when and how this should/can be used or which benefits/drawbacks it introduces?

Also, I imagine this increases the program memory occupation, in which case I ask if you can please update the relative memory occupation at the beginning of the README: I want this library to be usable on embedded systems, so I'm very conservative about anything which increases those values.

boothinator commented 2 years ago

Thanks for the feedback. I'll make some changes to address your comments.

boothinator commented 2 years ago

How are you determining that "580 bytes" number? Is there a specific piece of code that you're using to produce that number?

Also, I added a benchmark example where I'm directly comparing the performance of pass by value to pass by reference. One thing I noticed is that pass by reference is faster in every circumstance, even for something small like uint8_t. Would you consider making every argument a reference, regardless of item size?

That being said, I do want to go through the disassembly a bit to understand what GCC is producing better. I was expecting pass by value to be faster than pass by reference for small data types. Perhaps it's a quirk of the AVR architecture, or GCC, or the default optimization settings, or all of them.

rlogiacco commented 2 years ago

How are you determining that "580 bytes" number? Is there a specific piece of code that you're using to produce that number?

Output of the compiler when using the simplest example program, subtracting the space used by the stack... If I remember correctly... that was quite some time ago now

rlogiacco commented 9 months ago

Hey @boothinator , are you still willing to merge this PR in the main branch?

Would you consider making every argument a reference, regardless of item size?

Sure, can you foresee any undesirable effect for those using a previous version of the library?

boothinator commented 9 months ago

It's probably safest to leave things the way they are.