NuRaft icon indicating copy to clipboard operation
NuRaft copied to clipboard

Use unique_ptr for buffers

Open andy-yx-chen opened this issue 6 years ago • 6 comments

Another performance improvement is to use unique_ptr instead of shared_ptr for buffers, maybe worth to port as well, check this

andy-yx-chen avatar Nov 02 '19 07:11 andy-yx-chen

Thanks for your suggestion. Actually we plan to make the implementation of buffer pluggable, as adding a few bytes of meta in front of user data is disruptive (for those who want to avoid memcpy).

greensky00 avatar Nov 02 '19 16:11 greensky00

Thanks for your suggestion. Actually we plan to make the implementation of buffer pluggable, as adding a few bytes of meta in front of user data is disruptive (for those who want to avoid memcpy).

May I know when would this feature plan to be added as original one really hurts performance? Or are there any suggestions to minimize memcpy issue?

sheepgrass avatar Jun 11 '20 01:06 sheepgrass

@sheepgrass Currently, we don't have ETA as it is supposed to be a significant overhaul of the code.

memcpy issue exists only when your system uses its own buffer structure and tries to put the data in the buffer to Raft using append_entries(...) API. In such a case, you should allocate a separate ptr<buffer>, do memcpy from your buffer to ptr<buffer>, and then call Raft API with ptr<buffer>, which is quite redundant.

One possible workaround is using ptr<buffer> for your system as well, then you can directly put data to Raft without memcpy call.

greensky00 avatar Jun 11 '20 21:06 greensky00

@greensky00 Thanks for your reply. Actually, I came up with the workaround yesterday just similar to what you suggested. What I want to mention is that I want to minimize dynamic memory allocations as many as possible but using the provided buffer class would force us to use alloc(). To workaround this, I need to 'hack' my own buffer to have structure same as the provided buffer. I think it's a common scenario having data in a different structure other than NuRaft buffer.

And on a side note, if I got it right, calling append_entries() is a must when using NuRaft and I found that there are many dynamic memory allocations which I want to avoid.

sheepgrass avatar Jun 12 '20 06:06 sheepgrass

'hack' my own buffer to have structure same as the provided buffer.

Yes, this is the same way as what we are doing internally here.

Maybe we can provide a new overloaded append_entries() function, which accepts the general memory pointer and its size as parameters, and reduces the number of internal memory allocations. It will be a much smaller work than supporting generic/pluggable buffers.

greensky00 avatar Jun 15 '20 20:06 greensky00

Maybe we can provide a new overloaded append_entries() function, which accepts the general memory pointer and its size as parameters

Yes, it can help if such function can be provided. Beware that not to memcpy inside that append_entries() otherwise there is no point to have such function. (I think such approach is moving the 'hack' from outside of append_entries() to inside)

Also, be aware of the handling of log_entry inside append_entries() which may cause dynamic memory allocations and cause difficult to implement the new approach.

sheepgrass avatar Jun 16 '20 01:06 sheepgrass