blender
/
blender
134
400
Fork 584
Code Issues 6.3k Pull Requests 758 Projects 19 Wiki Activity

Core: new blenlib library for implicit-sharing #105994

Merged
Jacques Lucke merged 17 commits from JacquesLucke/blender:implicit-sharing into main 2023-03-28 13:58:02 +02:00
Conversation 3 Commits 17 Files Changed 10 +10 -3
2 changed files with 10 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ddc0ba8934 - Show all commits

11
source/blender/blenlib/BLI_implicit_sharing.hh
View File

@ -49,21 +49,28 @@ class ImplicitSharingInfo : blender::NonCopyable, blender::NonMovable {
BLI_assert(this->is_mutable());
}
/** True if there are other const references to the resource, meaning it cannot be modified. */
bool is_shared() const
{
return users_.load(std::memory_order_relaxed) >= 2;
JacquesLucke marked this conversation as resolved
Hans Goudey commented 2023-03-22 16:00:05 +01:00
Review
Copy Link

This could be over-documenting and redundant, I wonder what you think, but possibly some comments like this could help people answer their questions faster:

  • /** True if there are other const references to the resource, meaning it cannot be modified. */
  • /** Whether the resource can be modified without a copy. */
  • /** Call when creating another const reference/pointer to the data. */
    etc.
This could be over-documenting and redundant, I wonder what you think, but possibly some comments like this could help people answer their questions faster: - `/** True if there are other const references to the resource, meaning it cannot be modified. */` - `/** Whether the resource can be modified without a copy. */` - `/** Call when creating another const reference/pointer to the data. */` etc.
}
/** Whether the resource can be modified without a copy because there is only one owner. */
bool is_mutable() const
{
return !this->is_shared();
}
/** Call when a the data has a new additional owner. */
void add_user() const
{
users_.fetch_add(1, std::memory_order_relaxed);
}
/**
* Call when the data is no longer needed. This might just decrement the user count, or it might
* also delete the data if this was the last user.
*/
void remove_user_and_delete_if_last() const
{
const int old_user_count = users_.fetch_sub(1, std::memory_order_relaxed);
brecht marked this conversation as resolved Outdated
Brecht Van Lommel commented 2023-03-24 13:05:34 +01:00
Outdated
Review
Copy Link

My understanding is that user count decrement requires std::memory_order_acq_rel, while increment can use std::memory_order_relaxed.

For example libc++ does this for shared_ptr. More details:
https://stackoverflow.com/questions/48124031/stdmemory-order-relaxed-atomicity-with-respect-to-the-same-atomic-variable

My understanding is that user count decrement requires `std::memory_order_acq_rel`, while increment can use `std::memory_order_relaxed`. For example libc++ does this for shared_ptr. More details: https://stackoverflow.com/questions/48124031/stdmemory-order-relaxed-atomicity-with-respect-to-the-same-atomic-variable
@ -80,8 +87,8 @@ class ImplicitSharingInfo : blender::NonCopyable, blender::NonMovable {
};
/**
* Makes it easy to embed implicit-sharing behavior into a struct. This also allows the subclass to
* be used with #ImplicitSharingPtr.
* Makes it easy to embed implicit-sharing behavior into a struct. Structs that derive from this
* class can be used with #ImplicitSharingPtr.
*/
class ImplicitSharingMixin : public ImplicitSharingInfo {
public:

2
source/blender/blenlib/BLI_implicit_sharing_ptr.hh
View File

@ -13,7 +13,7 @@ namespace blender {
/**
* #ImplicitSharingPtr is a smart pointer that manages implicit sharing. It's designed to work with
* types that derive from #ImplicitSharingMixin. It is fairly similar to #std::shared_ptr but
* expects the reference count to be embedded in the data.
* requires the reference count to be embedded in the data.
JacquesLucke marked this conversation as resolved Outdated
Hans Goudey commented 2023-03-22 15:15:52 +01:00
Outdated
Review
Copy Link

expects -> requires, right? std::make_shared will allocate the use count with the object so it's helpful to clarify this a bit.

`expects` -> `requires`, right? `std::make_shared` will allocate the use count with the object so it's helpful to clarify this a bit.
*/
template<typename T> class ImplicitSharingPtr {
private: