Using C++11 smart pointers for asset manager

Is that a good practice?

It seems OK to me.

Is this more efficient than another solution, since i don't have to work on pointers manually?

No. Why would it more efficient? The point of smart pointers is that it is easier to get things right. It actually generally sacrifices a bit of efficiency in the process. But that's OK.

Also, what happens when i remove the Scene object?

The shared pointer gets destroyed, which means that the reference counter will be decremented. If it reaches 0 (which means no one else is using this resource), the Bitmap will be destroyed and its memory released.

So, it should work like a charm? Amazing!

Why are you using shared_ptr?

To me it seems "natural" that an asset manager "owns" the assets exclusively. It seems to be strange that an asset can outlive the manager, so no shared ownership should be required.

Having said this, unique_ptr is the more obvious solution to your problem.

Do not use shared_ptr unless you really need shared ownership, especially if efficiency is your concern.

Why are you using shared_ptr?

To me it seems "natural" that an asset manager "owns" the assets exclusively. It seems to be strange that an asset can outlive the manager, so no shared ownership should be required.

Having said this, unique_ptr is the more obvious solution to your problem.

Do not use shared_ptr unless you really need shared ownership, especially if efficiency is your concern.

I disagree.

Having a shared_ptr in asset manager and handing out weak_ptr to asset users is a solid strategy.

[Thats reply to cdoubleplusgood's post, in the meantime someone posted so just want to clarify as I didn't quote the original post ;)]

Question is - whats his plan to retrieve assets from manager? If he uses unique_ptr, he shouldn't really store any reference/pointer so he has to query manager every time he uses an asset, which will probably be every frame. He can't really pre-fetch assets that he uses and release them when he's done.

Also, when using shared_ptr manager knows when its the only owner left and can release resource (or keep it for some time before releasing completly).

How would unique_ptr be used in such pipeline?

Having a shared_ptr in asset manager and handing out weak_ptr to asset users is a solid strategy.

I don't understand this strategy. Why weak_ptr? The users will have to lock and check for expiration everytime using the asset. If you already have shared_ptr in the manager, why not for the users?

[Thats reply to cdoubleplusgood's post, in the meantime someone posted so just want to clarify as I didn't quote the original post ;)]

Question is - whats his plan to retrieve assets from manager? If he uses unique_ptr, he shouldn't really store any reference/pointer so he has to query manager every time he uses an asset, which will probably be every frame. He can't really pre-fetch assets that he uses and release them when he's done.

Also, when using shared_ptr manager knows when its the only owner left and can release resource (or keep it for some time before releasing completly).

How would unique_ptr be used in such pipeline?

I don't know what the OP's manager actually does and what he wants to achieve. Of course, using shared_ptr for automatic resource management can be very convenient. However, he asked for efficiency, and shared_ptr is not the most efficient way to maintain pointers, especially because shared_ptr is thread safe even if you don't need it: The reference counters are always synchronized.

Having a shared_ptr in asset manager and handing out weak_ptr to asset users is a solid strategy.

I don't understand this strategy. Why weak_ptr? The users will have to lock and check for expiration everytime using the asset. If you already have shared_ptr in the manager, why not for the users?

Because they're users. Giving out only weak_ptr's opens up the possibility to, f.ex. unload some resources that aren't actually being used in favor of some other high priority resources. This would need some mechnanism to avoid thrashing, tho. If you used shared_ptr instead, you wouldn't have the possibility to track actual usage.

Of course, using shared_ptr for automatic resource management can be very convenient. However, he asked for efficiency, and shared_ptr is not the most efficient way to maintain pointers, especially because shared_ptr is thread safe even if you don't need it: The reference counters are always synchronized.

This is certainly correct, and something to always keep in mind when copying around shared pointers. Passing around a shared or weak pointer is not close to a no-op as compared to passing around a raw pointer.

However, in the context of an asset manager you need to consider that the rather slow atomic increments/decrements used in the shared pointer are entirely neglegible compared to only a single disk access for loading an asset. We're talking of 30-50 nanoseconds versus 8-10 milliseconds.

Formally, giving out a weak pointer as suggested by Mnemotic is the correct thing to do (though giving out shared pointers would probably work just fine 90% of the time too), seeing how the manager owns the resources and should be allowed to toss them on a as-needed base, but safely so no resources "disappear" while in use.

I can't see how a solution that passes weak pointers would work in the case of multi-threaded code. In single-threaded code, I can check if my weak pointer is still valid before I use it, and I know that the object is not going away while I am using it. How do I get such a guarantee in multi-threaded code?

C++11 smart pointers are thread-safe. So sayeth the Standard.

I can't see how a solution that passes weak pointers would work in the case of multi-threaded code. In single-threaded code, I can check if my weak pointer is still valid before I use it, and I know that the object is not going away while I am using it. How do I get such a guarantee in multi-threaded code?

You're locking weak_ptr before use, which creates shared_ptr<> temporarily, so you have guarantee that it won't go away in that scope. Thats my understanding at least.

[Thats reply to cdoubleplusgood's post, in the meantime someone posted so just want to clarify as I didn't quote the original post ;)]

Question is - whats his plan to retrieve assets from manager? If he uses unique_ptr, he shouldn't really store any reference/pointer so he has to query manager every time he uses an asset, which will probably be every frame. He can't really pre-fetch assets that he uses and release them when he's done.

Also, when using shared_ptr manager knows when its the only owner left and can release resource (or keep it for some time before releasing completly).

How would unique_ptr be used in such pipeline?

Supposedly he would then be handing out raw pointers from the manager. Which is fine, it's usage of new/delete that's discouraged, not raw pointers.

You're locking weak_ptr before use, which creates shared_ptr<> temporarily, so you have guarantee that it won't go away in that scope. Thats my understanding at least.

Yes. You create a shared_ptr from the weak_ptr using lock, then you test if the shared_ptr is !nullptr.

I can't see how a solution that passes weak pointers would work in the case of multi-threaded code

It works by the combination of these two:

You're locking weak_ptr before use, which creates shared_ptr<> temporarily, so you have guarantee that it won't go away in that scope.

C++11 smart pointers are thread-safe. So sayeth the Standard.

Although the latter is a bit of a "relaxed" wording, it's close enough.

You are explicitly allowed to call all functions including copy constructor of a shared_ptr concurrently and you are guaranteed that the reference counter of a shared_ptr is at all times correct/consistent. That doesn't guarantee that the object still exists when/while you copy the shared pointer, of course. It does guarantee that the result is well-defined, however.

Locking a weak_ptr creates a copy of the shared_ptr which may be an empty shared pointer (if the reference count has gone to zero in the mean time and the object was destroyed). So you have no guarantee that your operation will succeed, but you are guaranteed that the result is verifiable. If the copied shared pointer is nullptr, locking the weak pointer failed and the object is gone. Otherwise you now hold a (temporary) shared_ptr and it's guaranteed that the object will live for as long as you hold the temporary.

Just giving some info to the discussion: I'm storing SDL_Texture's from calls made to IMG_Load and SDL_CreateTextureFromSurface subsequently. I fetch the textures from there to render with SDL_RenderCopyEx. I'm also storing Mix_Chunk and Mix_Music.

I find it a bit curious he used the phrasing "small asset manager" and suddenly we're all rambling about efficiency and what is pragmatically correct and it being multi-threaded and.. goodness do we even know if he cares about any of this stuff?

To be perfectly blunt performance loss from the internals of your asset manager are probably going to make an almost completely negligible effect on performance, the only important thing here is that you're not reloading a resource from disk unless you absolutely have to, how that works entirely depends on your game. For some games it makes sense to stream-load resources and unload a lot, for others you might just be sticking a bunch of junk in at the start of a level or something.

In the end even the naive approach of just loading everything at the start works fine for a great majority of games. Whats most important is you figure out what the thing needs to do to be easily usable from the outside.

Actually, i care about all those concerns. I might not implement strategy A or strategy B, but i would like to know them both. While my current project does not plan to multi-threaded, i would love to hear the opinion of more experienced developers towards my code and which challenges i might face with it. I'm enjoying the discussion, thanks everyone for all the insights!

I think I would start with shared pointers: It looks like it's the easiest solution to get right, and it should cover most situations. It's good to know about weak_ptr::lock(), but it seems cumbersome.