Simplifing dependencies/maintenance by removing `RCP` types

[oliverlee]

Would this project welcome a PR that replaces all RCP implementations with std::shared_ptr (and possibly allowing this to be swapped out with boost::local_shared_ptr)?

[isuruf]

I think the corresponding equivalent is boost::intrusive_ptr.

[oliverlee]

Possibly? I'm not very familiar with the boost smart pointer library but I did see RCP and some analog to enable_shared_from_this.

[rikardn]

As @isuruf mentions the RCP is implemented as an intrusive shared pointer rather than an std::shared_ptr. This is for performance reasons. There is a compile time option to not have thread safety that would mimic the use of boost::local_shared_ptr. Since we have our own implementation I don't know if we still need the dependency to Teuchos.

[bjodah]

I have a branch where I started to add yet another RCP type (which would solve problems with reference counting when interacting from the python bindings). I haven't gotten around to reviving that work, but I hope to in the future.

[oliverlee]

Is there a large performance hit from using std::shared_ptr?

Switching to a standard library implementation would decrease maintenance and possibly avoid reference counting issues from Python bindings. cppyy seems like it has out of the box support: https://github.com/search?q=repo%3Awlav%2Fcppyy%20shared_ptr&type=code

[bjodah]

I'm not convinced switching to std::shared_ptr would ease maintenance to any appreciable extent, and performance would suffer (benchmarks would be needed to quantify the slowdown though). But to me, those arguments are secondary. The header file I linked to contains information and links to nanobind's documentation that goes into great detail on the pros and cons of reference counting in c++ and its interplay with another reference counted language (in that case, python).

[oliverlee]

Feel free to close this as it seems that there isn't much interest in switching over to standard library types.

However I do think think it would reduce the need to debug issues such as https://github.com/symengine/symengine/issues/2048, std::shared_ptr comes with utilities such as std::allocated_shared (which helps with https://github.com/symengine/symengine/issues/2051 if ever pursued) and will possibly become usable within constant expression context in C++26.

[rikardn]

I have only anecdotal numbers for the speedup of using intrusive reference counting. Around 10-20% faster for every reference increase or decrease (because of one less indirection) and as much as 50% faster for creation (due to one less heap allocation). Since symengine do so many creations and reference count incs/decs this would add up. Of course this has to be benchmarked properly to be certain. I am not entirely sure but I think the constant expression context issue should not be applicable to the intrusive counter since it is not needing a separate heap allocation, so it will be similar to unique_ptr in this respect.