C++14 and SDL2: Managing Resources(ericscottbarr.com)

59 pointsby AndreyKarpovvor 12 Jahren44 comments

http://ericscottbarr.com/blog/2014/04/c-plus-plus-14-and-sdl2-managing-resources/

8 comments

This article starts with a flawed premise, that the right way to create a managed pointer is something like this[1].

    std::unique_ptr<void,void(*)(void*)> big_ptr(malloc(1), free);

The wasteful thing about this is that you have to store the deleter (free() in this case) right next to your void* for every pointer you manage, so the unique_ptr is twice as large as it needs to be. What you'd like to do is store the deleter in the type of the pointer rather than the pointer itself. It turns out unique_ptr can handle this. If the second template parameter is a type with an overloaded function call operator, then unique_ptr will use that to delete. We can wrap it up like so.

    template <class T, void F(T *)> struct unique {
        struct deleter { void operator()(T *p) { F(p); }; };
        typedef std::unique_ptr<T, deleter> ptr;
    };

With this helper type you can now declare a similar managed pointer

    unique<void, free>::ptr small_ptr(malloc(1));

but this time notice that the deleter function is the second template parameter and not an argument to the constructor and the following holds.

    assert(2 * sizeof(small_ptr) == sizeof(big_ptr));

[1] I used malloc() and free() so those of you following along at home can easily try this out for yourselves.

humanrebarvor 12 Jahren

> The wasteful thing about this is that you have to store the deleter (free() in this case) right next to your void* for every pointer you manage

In the examples given, the function pointers in question would be stored on the stack. How many windows should an application be juggling at once that an extra 8 bytes per window is a concern? This seems like a premature optimization to me.

If you're individually rendering blades of grass in a goat simulator, sure. But by the time you're dealing with arbitrary-length application-defined strings (like window names), an extra pointer isn't a big deal.

For me to believe that a small_ptr was worthwhile here, I would have to see some benchmarks, which is how good optimization happens anyway.

devcodexvor 12 Jahren

Thanks for the comment and suggestion. How might this look in the general solution though? The type of the resource has to be deduced somewhere and I wasn't able to figure out a good way to deduce that and declare this style of deleter within the std namespace.

I'm all for eliminating that extra bit of overhead and don't claim to be an expert, the whole purpose for me writing this article was to specifically learn about how to write reusable library code. If you have any suggestions for how to pull this off in the general solution I'm all ears!

Thanks again for taking time to comment!

[deleted]vor 12 Jahren

(1)

tdicolavor 12 Jahren

This is a clever use of unique_ptr, but I can't help but think this is missing the forest through the trees. There's a lot of difficult to read code so you can use a C library resource with a bunch of C APIs. If you wrap the window in a class you can get simple RAII semantics, and add a friendly more C++ API.

What if you want to add event handlers for window events, change the window title, or even (gasp) throw exceptions instead of constantly check return codes? Wrapping the resource in a class gives you a great place to add this functionality. Without the class you have a nice resource that won't leak but still feels like you're programming C.

michaelrmmillervor 12 Jahren

If you were creating a C++ wrapper, you still might prefer to use this same technique. Make the unique_ptr-wrapped resource a member of the wrapper instead of the raw resource. That would give the wrapper the same semantics as unique_ptr without any extra work. If you wanted shared ownership, use shared_ptr instead. Lastly, you wouldn't need to manually declare a destructor to free the resource.

In the end, I find it more readable to see unique_ptr and shared_ptr than raw resources. They describe more than just resource management.

simiasvor 12 Jahren

Wouldn't it be simpler to just wrap the SDL_Window C object in a C++ class? That seems like a lot of boilerplate to just save oneself a destructor call.

Also, I'm not usually one to bash C++ for its syntax (I've written quite a lot of C++ in my days and still use it from time to time) but my god I find that template spaghetti code physically painful to read.

plorkyeranvor 12 Jahren

The point of this is you can write it once and use it for every type rather than having to write a wrapper for each one.

simiasvor 12 Jahren

If I understand correctly you still have to write something like

    template<typename... Arguments> auto make_window(Arguments&&... args)
    {
        return detail::make_resource(SDL_CreateWindow, SDL_DestroyWindow, std::forward<Arguments>(args)...);
    }

for each resource.

Quite honestly at that point I'd sooner use a good old variadic macro, C99 style. I mean, sure, it's arguably less "elegant" and C++-ish but it'll be easier to understand, to maintain and won't spam me with pages of arcane template errors when I forget a colon somewhere, so it'll be easier to debug as well.

That's the double edged sword of the C++ template system I suppose, in order to gain in purity they lost the simplicity of the C macro system without gaining the expressiveness and convenience of the Lisp macro system. In the end, what percentage of C++ coders use templates outside of the STL you think?

pjavor 12 Jahren

The C++ template will give you extra type checking that the varadic macro won't, which is worth the tradeoff for the obscure error messages for me.

(At least the errors have got slightly better in recent g++ releases with colourised error output.)

masklinnvor 12 Jahren

> If I understand correctly you still have to write something like

You don't have to, you can create further helper function calling the generic thing but it's optional.

devcodexvor 12 Jahren

The code that makes use of templates and such is meant to be library code in the same way that STL is, not something the average user interacts with other than to know they need to pass an Init and Destroy function followed by initialization parameters. I don't see people tearing apart <tuple>'s code because of its complexity, or <vector> or any other reusable STL component that most developers who've complained about the verbosity of my solution use every day without batting an eye.

In these proposed solutions I've not yet seen one that has the capability to do this:

    auto window = make_resource(SDL_CreateWindow, SDL_DestroyWindow, ....);

With my solution all you have to do is call make resource with different parameters it automatically works with other C style init/destroy created resources, not just from the SDL library but any C library that follows that practice.

Can you try posting what your solution to the general problem would look like?

angersockvor 12 Jahren

The thing is, you're going to be wrapping in a class anyways, so why not move all the nonsense into a normal destructor and then use normal smart pointers?

I suggest that this is somewhat solving the wrong problem.

10098vor 12 Jahren

This, and also it would bother me a lot to know that I'm essentially re-implementing what is already available in the standard library.

forrestthewoodsvor 12 Jahren

My god. That's a hideous amount of hard to read code to support such a simple operation. Do not like. At all. (And I write C++ video game code all day every day)

nanofortnightvor 12 Jahren

This is meant as library code, not application code, though for template work, this is actually quite lightweight. You'd see scarier creatures than this if you write C++ libraries. Perhaps you should take a look at the implementation of your standard libraries or your favourite boost library sometime.

Let's break it down:

If this weren't library code you mightn't care about proper forwarding (which isn't important anyway since the function you're calling is written in C and C doesn't have lvalue-references!! - unnecessary complexity), so you would sloppily write this as:

    template<typename Creator, typename Destructor, typename... Arguments>
    auto make_resource(Creator c, Destructor d, Arguments&&... args)
    {
        typedef decltype(*c(args...)) ResourceType;
        return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(c(args...), d);
    }

Which is much easier to understand, yes? Your standard passing multiple arguments along thing like you'd normally do when writing a custom printf.

Actually, now that I've had time to think about it, you can simpify the original code:

    template<typename Creator, typename Destructor, typename... Arguments>
    auto make_resource(Creator c, Destructor d, Arguments&&... args)
    {
        auto r = c(std::forward<Arguments>(args...));
        typedef typename std::decay<decltype(*r)>::type ResourceType;
        return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(r, d);
    }

And again, written sloppily:

    template<typename Creator, typename Destructor, typename... Arguments>
    auto make_resource(Creator c, Destructor d, Arguments&&... args)
    {
        auto r = c(args...);
        typedef decltype(*r) ResourceType;
        return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(r, d);
    }

Or, if you don't mind repetition of ResourceType:

    template<typename Creator, typename Destructor, typename... Arguments>
    auto make_resource(Creator c, Destructor d, Arguments&&... args)
    {
        auto r = c(args...);
        return std::unique_ptr<decltype(*r), void(*)(decltype(r))>(r, d);
    }

Clearer?

pbsdvor 12 Jahren

The standard dictates that decltype(*r) deduces to T&, not T. The std::decay is there to collapse the reference type into a value type. Therefore your simplification results in a compilation error.

An alternative way to get the value type is to use std::remove_pointer_t:

    using ResourceType = std::remove_pointer_t<decltype(c(args...))>;

devcodexvor 12 Jahren

Thank you for taking the time to write this out, you hit the head exactly on what I was trying to accomplish with this article. I saw an opportunity to make a reusable tool for myself while at the same time being able to work on some of the best practices I've seen for developing library code. The real payoff is in the usage code.

Your simplification of the original code is great! I like how pulling out the creation of the the resource to its own line makes the typedef and unique_ptr initialization much easier to read (as far as templates go that is, a real sticking point here today).

I have updated my article and attributed the clarification to you, thanks again for your feedback and comments!

azthvor 12 Jahren

I suppose you simply wrap the functionality in a class with a destructor? That's what I thought at first too; however, the author mentions in the comments section on that page something about this interacting in a hard fashion with C++11's rvalue references, and copy and move constructors.

Another use on here suggested a shorter and more efficient way to write the code [0].

https://news.ycombinator.com/item?id=7610571

10098vor 12 Jahren

Maybe this one is simpler:

http://onlinehut.org/2013/01/smart-pointers-are-not-just-for...

though I was trying to use that method to manage handles, not pointers. I eventually had to write a wrapper for handles for it to fulfill the requirements imposed by unique_ptr.

devcodexvor 12 Jahren

The function that does all the work is 5 lines and then it works for any resource created from Init/Destroy function pair (a very common practice in C libraries), that's a hideous amount of code to you?

With your experience, how would you write a general solution for this problem?

angersockvor 12 Jahren

Yeah, honestly, I'd prefer our smart pointer class over that nonsense any day.

You end up exposing the SDL functionality as a normal class object, and everything's much more straightforward.

nlyvor 12 Jahren

unique_ptr is not really a good solution to this problem. Something like Boost.ScopeExit for short lived resources, or shared_ptr (which stores the deleter using type erasure, meaning you don't need to allow evidence of the deleter to to permeate through your codebase) is a lot easier on the eyes...

    template <typename... Args> inline
    std::shared_ptr<SDL_Window> CreateWindow (Args&&... args) {
        return std::shared_ptr<SDL_Window> {SDL_CreateWindow(std::forward<Args>(args...)),
                                            &SDL_DestroyWindow};
    }

    auto window = CreateWindow ("App Name", SDL_WINDOWPOS_UNDEFINED, 
                                SDL_WINDOWPOS_UNDEFINED, 640, 480, 0);

You know, or someone should write a good quality wrapper like you would just expect for any other language (Python, Ruby, whatever). Many here seem to be blasting C++ when nobody is doing any better without a similar level amount of work elsewhere.

lholdenvor 12 Jahren

'shared_ptr' can use type erasure because it is a much heavier object. 'unique_ptr' on the other hand is supposed to be a nearly free.

If you wanted a version of your code with unique_ptr, you would create a SDLDelete struct template. The result would be nearly identical except for an additional type parameter (and the new struct).

Then if you wanted to make "CreateWindow" generic for any of SDL's create/delete pairs... you are basically looking at the OP's make_resource code but slightly different. :P Their version just avoids making a new struct.

aktauvor 12 Jahren

I can barely imagine a more fitting love sonnet for good old (modern) C99/11. My eyes, dear $deity my eyes. I'm sure OP is a fantastic(ally) smart dev but I'm kind of glad I don't have to work with that.

dfbrownvor 12 Jahren

This is no worse than anything else in the standard library. Writing generic template code is always ugly but once written you don't have to deal with the ugliness to use the resulting functions. It's not worth the complexity in most cases but if it's something general you're going to use a lot (and lots of C libraries have this idiom of Resource_Init Resource_Destroy function pairs) then it can be worthwhile.

Eigen is a quintessential example. It's some of the most complex and inscrutable template code out there but it makes writing numerical code in C++ almost as nice as matlab while also being extremely fast.

mattgreenrocksvor 12 Jahren

Except it's not common code at all.

Want to look at some tasteful C++ code? Check out LLVM.

devcodexvor 12 Jahren

Haha I wouldn't call myself a smart dev, just one that's trying to learn how to write good library code. If you really want to make your eyes bleed look at the proposed implementation of just such a utility:

http://isocpp.org/files/papers/N3949.pdf

Behind that wonderful STL code lies some crazy code behind the scenes. Is it beautiful? You won't catch me taking that argument! But is it typesafe with minimal overhead and easy for the end user of the code to understand and use? That's the one that's most important. For all the complexity of the template function itself, using it comes down to this:

    auto window = sdl2::make_window("App Name", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 640, 480, 0);

Not an angle bracket in sight!

masklinnvor 12 Jahren

Well the ugliness here comes from the creation of a completely generic wrapper generator for any (constructor, destructor) C pair. If you just want to wrap one pair, you can typedef on unique_ptr and you're done

okamiueruvor 12 Jahren

My own way of doing this:

https://github.com/swarminglogic/sdl2-cross/blob/master/src/...

https://github.com/swarminglogic/sdl2-cross/blob/master/src/...

Certainly less verbose, and solves the problem quite nicely as far as I've been using it. If someone with some wisdom feel like commenting on it, I'm not averse to learning.

mattgreenrocksvor 12 Jahren

Wrapper classes + RAII is superior to this approach:

1. Easier to read

2. Easier to use

3. Doesn't rely on bleeding-edge compiler features

I used to do some fun stuff with templates back in the day (CRTP), but I had trouble reading this.

Usually, the number of discrete resources you have to wrap this way is fairly small. I tend to cheat and write header-only libraries because the classes are tiny, don't change much, and shouldn't be used in every compilation unit if you have any semblance of a decent architecture.

Edit: clarified that I mean wrapper classes due to nerd sniping

masklinnvor 12 Jahren

> RAII is superior to this approach:

unique_ptr is not RAII now?

> 1. Easier to read

That's debatable, here ``make_resource`` has to be understood once and will thereafter work with any C constructor/destructor pair. Considering the tone of your comment I guess you mean wrapping each pair in its own object so each of these wrapper objects (and its ownership semantics) has to be understood, and at a fundamental level they're redundant with unique_ptr.

If you don't like the generic thingy and want one wrapper object per call pair, you can just typedef the unique_ptr.

> 2. Easier to use

Easier to use than calling a function and having a unique_ptr with clear ownership semantics?

mattgreenrocksvor 12 Jahren

Don't appreciate your tone. Relax buddy, it's Friday.

I'm arguing for introducing thin abstractions over the underlying API to handle lifetime issues, and encapsulate icky, SDL-specific details and types as they arise.

They are definitely redundant with unique_ptr. In return, they're higher-level and more abstract. It's up to the developer to select what level they need to be at. I favor abstraction and readability, others may not.

masklinnvor 12 Jahren

> Don't appreciate your tone. Relax buddy, it's Friday.

Wow, sorry, I didn't know you were an internet badass, that changes everything.

As for relaxing, have you considered taking your own advice?

> I'm arguing for introducing thin abstractions over the underlying API to handle lifetime issues, and encapsulate icky, SDL-specific details and types as they arise.

Yes, now that you point it out I can certainly see a careful balancing of the tradeoffs between having to maintain custom code versus the ability to create more extended abstractions in your summary declaration that unique_ptr is hard to read, hard to use and not RAII.

dangvor 12 Jahren

The comment you responded to was indeed inappropriately irritating. But please don't respond to incivility with more incivility on HN. It's hard not to, but it only makes things worse.

devcodexvor 12 Jahren

std::unique_ptr is a class that wraps around a pointer for the express purpose of providing RAII semantics. How is it not a wrapper class? All you're proposing is writing a class that does what unique_ptr does only now without templates each type has to be written out by hand.

Do you not ever make use of any of the STL templates like std::vector because their implementations are hard to read? Probably not, because they are easy to use and you never have to worry about how they are implemented. Well, here is how you use my function, please tell me how I could make this function call any easier because I honestly would like to know.

auto window = sdl2::create_window("New Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, 0);

I wrote a short function that I can reuse with any C based Init/Destroy created resource. Now that it works I never have to think about any of the template magic that went into writing it because using it is dead simple... there aren't even any angle brackets in the call. Not a single proposed "better" solution in this thread has shown the capability of creating resources in the general manner, each one shows the creation of a concrete class which means it has to be duplicated for each type. Which defeats the point of the article.

If you worked with templates back in the day then you probably aren't too familiar with the parameter pack syntax or auto return type deduction, these are new things and yes they are bleeding edge. How is anyone suppose to understand and learn good practices for using these features if they don't make use of them and write about it for others to provide feedback on?

I wrote this article to learn about specific techniques and get feedback on them. It's C++, there's a million ways to skin a cat here... and this article was specifically to address C++14 features. That's why it wasn't titled "C++03 and SDL". I thank you for taking the time to comment but telling someone they shouldn't make use of new compiler features because you aren't familiar with them and then not providing any sort of example yourself to the proposed problem doesn't come across as constructive at all.

tdicolavor 12 Jahren

I agree with you, I would personally stick with a wrapper class unless there was some ungodly number of resources to wrap. There are very likely other operations on a window object which will benefit from being able to wrap it in a class with more points for extension.

10098vor 12 Jahren

that is RAII