Remove ref_ptr

[scheibel]

With https://github.com/hpicgs/globjects/pull/275 the discussion comes up why we provide an own implementation of a smart pointer (ref_ptr), if the STL provides unique_ptr, shared_ptr and weak_ptr.

Typically, objects in globjects are strictly related to an OpenGL object, so there is a 1 to 1 correspondence and with a creation or destruction of a globjects objects there should follow the same for the OpenGL object. As objects (e.g., a shader) can be used in different ways by multiple other objects (e.g., multiple programs sharing the same fragment shader), it may not be clear when an globjects object isn't used anymore and the corresponding OpenGL resource can be freed. The problem gets even more complicated when we want to allow other OpenGL libraries to use OpenGL objects allocated by globjects (and the other way around). Therefore, our first solution was to introduce a shared ownership architecture, as it can be established using shared_ptr or ref_ptr. ref_ptr has the main advantage over shared_ptr that the reference counting is on the object itself, resulting in smaller and clearer function signatures. The disadvantage is the required own implementation, as a ref_ptr is not shipped with the STL (by now?).

While analyzing past and current rendering systems (e.g., research prototypes, game engines, cg demos) it becomes apparent that there is not only a common phase for creating OpenGL objects but also a dedicated phase were OpenGL objects are freed. As I assume that every programmer using OpenGL knows (or at least should know) when to allocate and free OpenGL memory, it feels prematurely optimized that we introduced a shared memory ownership and management in the first place.

I suggest we switch back to simple, straight-forward memory management using unique_ptr (indicating memory ownership) and plain pointers (indicating no memory ownership), as seen in http://stackoverflow.com/questions/8338570/c11-replace-all-non-owning-raw-pointers-with-stdshared-ptr#answer-8376611.

[mjendruk]

One other problem I see with the current approach is that every globjects object has to be created on the heap, separately. That ruins caching mechanisms. Often, a drawable consists just of a couple of integer IDs: a program ID, a VAO ID, maybe shader IDs etc. To be cache efficient, these should be at the same position, but because of the current implementation, everything is spread across the memory. Therefore, there should be the possibility to create objects on stack or as class members.

[scheibel]

We allowed this in the beginning of globjects/glow and the very first issues we had to solve was the incorrect moment users of globjects instantiated objects (e.g., as no OpenGL context was active) so we decided to disallow this error-prone usage. On the other hand, typically I support the idea of not restricting developers in the usage of a library/class with the implication that the code might break if the user just don't use it right.

I think we may switch back and allow instiantion of globjects objects everywhere and I would look forward to cleaner code when we can remove HeapOnly and the unintuitive release interface for its subclasses, but I don't want to decide this alone.

[j-o]

I think it is generally a good idea to let the programmer have their freedom; however, the ownership rules must be clearly stated when OpenGL objects belong together, e.g., who owns a shader? The program(s) it belongs to or whoever created it? Same thing goes for VertexArray and VertexAttributeBinding, and Framebuffer and FramebufferAttachment. Personally, I found it very useful to have globjects manage these itself. For some of these cases shared ownership might be an appropriate solution to keep the current level of convenience.

[scheibel]

I started a ref_ptr-free globjects implementation yesterday and I hope to finish it today so you can have a look at it. It's true, that the ownership is an actual problem when removing ref_ptr and as I stated in my first post, std::shared_ptr ownership is a problem with third-party libraries.

For your specific examples I have the current solution:

• Program owns no shader
• VertexArray owns its bindings
• Framebuffer owns its attachments

A little bit more difficult it is for objects that can either use a given object or create one by itself if needed (e.g., ScreenAlignedQuad with the fragment shader and the program constructor)

My current straight-forward solution is that the creating source code has the ownership and for now I haven't needed ownership-passing.

[mrzzzrm]

Actually I really like the approach of self-contained ref-counting (i.e. Referenced), it's not an uncommon thing to have and it's very convenient. The only problem I have with it is the fact that we could potentially use std:: features to emulate similar behaviour and thus stick with unnecessary code-overhead at the moment.

@cgcostume mentioned std::enable_shared_from_this. I think it would be a valid alternative, even though you can't call std::shared_ptr<T>(t) to increase the ref-count. If we're really concerned about reducing "boilerplate" and allowing globjects-users use the stdlib pointer they are (hopefully) used to, why not, let's go for it. It would be a little more cumbersome to use at the beginning, that's all.

We allowed this in the beginning of globjects/glow and the very first issues we had to solve was the incorrect moment users of globjects instantiated objects (e.g., as no OpenGL context was active) so we decided to disallow this error-prone usage.

I think it's useless to protect people against themselves. I don't think we should restrict usage for everyone just to patronize a minority.

One other problem I see with the current approach is that every globjects object has to be created on the heap, separately [...]

Heap doesn't mean fragmentation (I assume that's what you mean when saying "separately") If you want to manage the memory layout yourself, you can (http://www.parashift.com/c++-faq/placement-new.html). (With the current limitation that that memory has to be on the heap though, granted)

EDIT: that being said it would of course be way more convenient of it would be possible to directly use instances of globjects as members. So yeah, I don't think HeapOnly is the best of all ideas ;)

[scheibel]

A globjects without ref_ptr can be examined here: https://github.com/hpicgs/globjects/commit/be9aa3575a18c51a371a7079cedf7226b4c08c0b

Now I'll have a look at a globejcts without HeapOnly.

[scheibel]

And a globjects without HeapOnly can be seen here: https://github.com/hpicgs/globjects/commit/9207c3f873a62836bd038d10ea73d44f5d62bc27

[scheibel]

As I'm currently revisiting the complete globjects code to take full advantage of std::unique_ptr and to validate the ownership, I found several places where we have memory leaks...quite interesting.

[j-o]

How about letting the programmer decide who owns an object? A possible solution might be to provide two versions for methods taking another object:

• one taking a std::unique_ptr via move semantics, thereby transferring ownership
• and another one taking a raw pointer, leaving ownership where it is

Storing these pointers could be encapsulated in wrapper, so that constructs with m_object and m_ownObject (like in ScreenAlignedQuad) won't be necessary.

[scheibel]

I think this would really bloat up the interfaces and I'm not even sure if this is applicable everywhere. Additionally, I can't think of a solution where we don't mark pointers as owned or not owned. But it doesn't have to be a duplicated pointer (one holding a unique_ptr, the other one is raw), it can also be an object that wraps a raw pointer and the ownership information.

Hopefully we come to a decision throughout the next week.

[mjendruk]

I agree with @scheibel. Also, just to give some further input. Maybe it could work with these rules:

1. For members: if it can be set by the user in any way, make it a non owning raw pointer. This way, objects can be both used by just one and many other objects without the overhead of shared_ptrs. The user has to take care of the destruction in any way.
2. Use unique_ptr for any member that is created and managed inside of library classes.
3. For classes like ScreenAlignedQuad where members can be user-set as well as default implementations, apply rule number 1 and make the default objects accessible to the user in a way that he can pass it just like he would do it with his own objects, e.g., through static methods that return unique_ptr.

I think a similar approach is used by bullet physics.

Besides all that, I think that any solution without shared_ptrs will make globjects less convenient to use.

[scheibel]

Besides all that, I think that any solution without shared_ptrs will make globjects less convenient to use.

Do you actually mean shared_ptr or do you talk about std smart pointers in general?

[mjendruk]

Any kind of reference counted smart pointer.

[j-o]

Concerning @mjendruk's rules: I think we can all agree on the second rule; however, with the first rule I feel globjects would be less convenient as the users would have to manage a lot more objects themselves.

In any case, if we end up having the possibility of own and not-owned objects, this should be wrapped in a class and @scheibel's suggestion is probably better here.

A litte improvement to my last suggestion: The wrapper could be implicitly constructable from both raw and unique pointers, so duplicated methods won't be necessary anymore.

[cgcostume]

It is not much of a problem in having simple "default" c++11 applied to our OpenGL object wrapping classes. Most/all these classes do not need to own other globjects, just to work with them. So we need to decide on which level we want to ensure the existence of used, not-owned objects: (1) move problem to user, use raw pointers for non-owning members (@mjendruk) (2) enforce use of shared_ptr and weak_ptr in the public interface of opengl object wrappers (3) or leave it as is.

The problem now - we need some/more users to decide which would be the best but imho it is not (3), but either (1) or (2). Furthermore, since the integration into large, existing rendering frameworks is possible, but not the core use-case of globjects, (2) is equally valid to (1).

I prefer (2), since the programming of OpenGL can be demanding and every bigger program not using memory management usually has mem-leaks, especially when no rendering framework is used. When applying for (2), globjects users get the idea of the API - "i have to use managed pointers for opengl wrappers". They still can, e.g., wrap smart stuff around it, do arbitrary memory management, ... , create memory leaks in their own code (c++ will always provide the means to destroy its good intentions). But at least, we show that we prefer to enforce managed memory for globjects. And it's in line with latest "C++ defaults" i think (i.e., A Tour of C++). A heap only requirement however, gets irrelevant then (as suggested by @scheibel).

Still: does globjects need to enforce 1:1 mapping of OpenGL objects? I think it should not - an instance of a OpenGL object wrapper class (globject) could provide access/handle to the capabilities of an OpenGL object. Perhaps we should work out some requirements and see which of these we can satisfy, e.g.,

• handles should only be valid, as long as their objects are valid (hard to enforce in mixed use - use with rendering frameworks, but easy if only those handles are used)
• handles to the same object should be equally valid (that does not require for same object in memory)
• ...

[mjendruk]

I am also for option (2) since as @j-o and I already said option (1) would be way less convenient. The shared_ptr still allows enough flexibility through deleters and allocators.

Some thoughts on having raw and smart pointers: Supporting smart and raw pointers by abstraction doesn't sound like a good idea to me, because it implies a virtual function call each time a wrapped object is accessed. I had concerns about the use of shared_ptrs because of performance reasons, such wrapper however would be even worse.

Edit: If the user can't use the shared_ptrs in his code, because he has an own implementation etc., he can still put a custom deleter that does nothing into the shared_ptr.

[mrzzzrm]

What's the status of this?

[scheibel]

We don't have finally decided this by now, but we strive for removing ref_ptr, working out a concept of when a user has to manage pointer ownership by himself (e.g., creating a program normally means you have to store the pointer and you have to release it) and for what cases we have to decide on established smart pointers or raw pointers for internal memory management (e.g., for the AbstractStringResource instances that are normally not created or held by the user).

It's a little bit more complicated since this decision also affects gloperate and other dependent projects that uses ref_ptr as a simple memory management mechanism.

[marton78]

Hi guys,

I just stumbled upon your library and am considering using it in my own projects to replace similar code I've written myself. While I generally like your work, the first things that made me suspicious is

1. The use of naked pointers and new in your examples (where you could simply use automatic memory management, i.e., allocating objects on the stack)
2. The existence of ref_ptr

I strongly suggest you remove ref_ptr as well as the embedded reference count from the library. Memory management is entirely orthogonal to graphics and thus out of scope of a GL wrapper library. Therefore, I suggest that GLobjects be agnostic of memory management.

A sane design would be that of the move-only types in the STL, e.g. std::thread. That is, a class should be

1. non-copyable
2. move-constructible and move-assignable
3. have an empty state in which destruction is a no-op

This way, questions of ownership can be decided by the user:

1. Unique ownership: Allocate object on stack, pass around references (not pointers!) to it and maybe move it (unique_ptr does exactly this, but is not needed in this case, if the objects themselves are moveable)
2. Shared ownership: keep shared_ptrs constructed by make_shared thereby embedding the reference count into the object.

BTW, in my projects I have never encountered a true use case for shared ownership, to me the use of it indicates bad design, but that should be the concern of your users, not yourselves.

Hope that helps somehow, keep up the good work, Marton

[scheibel]

My current findings are that we should clearly separate between objects that can be created by the user and the ones that shouldn't be created by the user. Giving this separation we can ensure that we have full memory ownership and delete responsibility over objects that cannot be created by the user and that we can ignore the delete responsibility for each object that has to be created by the user.

For example, the separation can look like this:

Create by user:

• AbstractStringSource and subclasses
• AbstractState
• AbstractUniform and subclasses
• Object and subclasses
• NamedString

Create by library:

• AbstractFunctionCall
• AbstractLogHandler
• FramebufferAttachment and subclasses
• Capability
• StateSetting
• VertexAttributeBinding

This separation would require to remove all convenience interfaces that allow to omit the direct instatiation of objects from the first group (e.g., passing a std::string to the constructor of Shader, internally creating a StaticStringSource object).

Further, we can provide additional (user-instantiated) classes in depending libraries that can wrap user-instantiated objects with the responsibility to delete them.

This solution would implement the scenario of a memory-management agnostic OpenGL wrapper from @marton78 (thanks for your insights).

[scheibel]

I close this for now as we have to decide internally on how to proceed with this issue.