Shared_ptr, unique_ptr, ownership, can I overdo it in this particular case?

Question

I work on graphical applications and use generic and unique pointers, essentially because it handles freeing memory for me (as well as convenience), which is probably bad (if that's the reason I use them).

I recently read an answer to a question about Stackoverflow, which mentioned that, according to B. Straustup, unique / general ptrs should not be used at all, and that arguments should be passed by value.

I have a case in the chart for which I think using shared_ptr makes sense, but I would like to know from experts (I am not an expert in C ++), if I took up / thought about it, and if so, then what will they do instead (to be consistent with C ++ recommendations and efficiency)

I will take a general problem arising in Rendering / Ray-Tracing. In this specific task, we have a pool of objects (we will use triangles for this explanation) and a structure, which for simplicity of explanation we will call a regular three-dimensional grid. Let's say that at some point we need to insert triangles into the grid: what does this mean that we need to check the bounding volume of each inserted triangle, overlapping any of the cells from the grid, and this happens, then each overlapping cell needs to save a pointer / link to this triangle (for later use). A triangle can overlap more than 1 cell, so multiple cells can be referenced by several cells (you see where I am going with shared_ptr here).

Note that outside the grid structure we do not need a pool of triangles (so technically the object that owns the triangle pool here is a grid or, more precisely, grid cells).

 class Grid { struct Cell { std::vector<std::shared_ptr<const Triangle>> triList; }; void insert(triangle*& tri_) { std::shared_ptr<const Triangle> tri = tri_; for (each cell overlapped by tri) { // compute cell index uint32_t i = ... cells[i].triList.push_back(tri); } } Cell cells[RES * RES * RES]; }; void createPoolOfTrianglesAndInsertIntoGrid() { Grid grid; uint32_t maxTris = 32; Triangle* tris = new Triangles[maxTris]; // process the triangles ... // now insert into grid for (uint32_t i = 0; i < maxTris; ++i) { // use placement new Triangle* tri = new (&tris[i]) Triangle; grid.insert(tri); } // no need to delete tris here ... it should be done by // the grid when we go out of this function scope } 

It sounds complicated, but my motivation for this design is to follow the Stroustrup recommendation. In this case, the createPoolOfTrianglesAndInsertIntoGrid function createPoolOfTrianglesAndInsertIntoGrid not own triangles, this is done by the mesh cells. Therefore, I allocate memory in the createPoolOfTrianglesAndInsertIntoGrid function, because I need to create triangles here, and then I use the new placement method to get a pointer to each triangle in this pool, which I can then transfer to the insert grid (I cancel the memory management of this object by this method). There, it converts the triangle to shared_ptr , and cells can now share a link to it (using shared_ptr ).

I wanted to know, in your opinion, this is happening in the right direction, or if it seems completely wrong, both from the point of view of implementation and from the point of view of a possible loss of efficiency (I allocate a memory pool and then use a new place to create a temporary triangle which I then pass to the grid insertion method, then convert to shared_ptr, ...)

I try to both learn and improve my code and hope that you can provide valuable professional feedback.

EDIT: I am basically trying to find the right approach to this problem. + I will try later to make some changes based on your comments.

Answer 1

It seems to me that your Grid owns triangles. I assume the triangles are relatively light (3-5 dimensions?).

I would think this might work. I use the container in the Grid to grab triangles by value . The container will remove the triangles when the Grid goes beyond.

Then each Cell simply uses raw pointers to keep track of which triangles it refers to. Cell do not have triangles; they simply hold pointers to triangles belonging to the Grid .

 class Grid { struct Cell { std::vector<Triangle*> triList; // non owning }; void insert(Triangle tri) // pass by value { tris.push_back(tri); // Grid owns this by value for(each cell overlapped by tri) { // compute cell index uint32_t i = ... cells[i].triList.push_back(&tris.back()); } } // Use a deque because it won't re-allocate when adding // new elements to the end std::deque<Triangle> tris; // elements owned by value Cell cells[RES * RES * RES]; // point to owned elements }; void createPoolOfTrianglesAndInsertIntoGrid() { Grid grid; // owns the triangles (by value) uint32_t maxTris = 32; std::vector<Triangle> tris(maxTris); // process the triangles // ... // now insert into grid for(auto tri: tris) grid.insert(tri); } // no need to delete tris here ... it should be done by // the grid when we go out of this function scope } 

NOTE. I use std::deque to store triangles by value in a Grid . This is because it will never redistribute its internal memory when adding new triangles. If you used std::vector , your source pointers would end up breaking when std::vector resized.

As an alternative:

Given that you are building all your triangles in your function, and then passing them all to the Grid , why do it all at once? You can transfer the entire container in one go. If you do this using move semantics, you don’t even have to copy anything:

 class Grid { struct Cell { std::vector<Triangle*> triList; // non owning }; // Accept the entire container in-tack // (no reallocations allowed after this point) void insert(std::vector<Triangle> tris) // pass by value (able to move in) { // for(auto& tri: tris) { for(each cell overlapped by tri) { // compute cell index uint32_t i = ... cells[i].triList.push_back(&tri); } } } // Using a vector so it MUST NOT be resized after // Cells have been pointed to its elements!!! std::vector<Triangle> tris; // elements owned by value Cell cells[RES * RES * RES]; // point to owned elements }; void createPoolOfTrianglesAndInsertIntoGrid() { Grid grid; // owns the triangles (by value) uint32_t maxTris = 32; // Build the triangles into std::vector<Triangle> tris(maxTris); // process the triangles // ... // now insert into grid grid.insert(std::move(tris)); // move the whole darn container (very efficient) // no need to delete tris here ... it should be done by // the grid when we go out of this function scope } 

NOTE. Now I used std::vector because you do not add triangles one by one after they come to the Grid . But you SHOULD make sure that the owner of std :: vector does not change inside the Grid .

Answer 2

Efficient memory allocation is a separate issue for the life of the object and the behavior of the object.

The distribution strategy management mechanism is Allocator in std::vector<Type, Allocator> and in std::allocate_shared<Type, Allocator>

It looks like you want to perform allocations from the pool.

Firstly, providing the semantics of the Triangle value (i.e. does not contain a pointer to it) will allow std::vector efficiently allocate memory blocks already. A distribution strategy assumes that more than one facility is required. Under the covers, it allocates memory blocks and calls the corresponding constructors / destructors in place.

If Triangle really needs to be a shared descriptor (i.e. from the point of view of shared_ptr), you can allocate shared_ptr using a custom allocator.

Boost has some examples of memory pool allocators.

Answer 3

If the cells are the sole owners of the triangles, then you do not need a pool, just select the triangles as shared_ptr and paste them into the grid. Thus, all triangles that no longer belong to any cells will be cleared.