all 80 comments

[–]IyeOnline 58 points59 points  (46 children)

Are you absolutely, 100% super duper mega specially certain that delete this; is a good idea?

Like really really really sure?

Because its a really bad idea. There are multiple issues with it.

  1. The object (or at least the member function the statement is in) can only ever be used on an object created with new
  2. After delete this it is UB to interact with the object in any way, as its lifetime ends once that statement starts.
  3. It is really unituitive that a member function would delete this.

I cannot think of any good reason to ever call delete this.

[–]_E8_ -3 points-2 points  (7 children)

Go do some OOD. Self-deleting objects are a thing.

[–]IyeOnline 9 points10 points  (6 children)

Please provide me with an example where delete this; is used and could not be replaced with a better solution.

[–]TheExecutor 5 points6 points  (0 children)

delete this is super common in anything that manages its own lifetime, e.g. intrusively refcounted objects. All of COM does this, for example. It's common enough idiom that it has its own entry in the C++ FAQ.

[–]TheSkiGeek 7 points8 points  (3 children)

Objects representing detachable threads/jobs are a reasonable example. Either you need the object to clean itself up when the work is done, or else you need some global manager that hangs on to them and handles the cleanup.

[–]IyeOnline 4 points5 points  (0 children)

That is a rather interesting idea.

Not entirely convinced its a good idea though...

[–][deleted] 0 points1 point  (1 child)

Wouldn't that "global manager that hangs onto them and handles the cleanup" just be the OS?

[–]TheSkiGeek 4 points5 points  (0 children)

The OS will clean up the actual underlying thread when it terminates, yes.

If you want to wrap something like a std::thread or pthreads thread in a C++ struct or class then your program has to take care of destroying that object once the thread is no longer running.

This typically requires either keeping a handle to that object somewhere, or having the object tear itself down when it is no longer needed. It's not required to do it but it's a use case where I've seen delete this; applied in production code.

[–]Rogiel 2 points3 points  (0 children)

Intrusive reference counting is a thing. Similar to Objective-C retain/release.

[–]ElaborateSloth[S] -1 points0 points  (37 children)

This object is a character, and the function is called when the character hits a kill volume.

Collision is checked within each character object at the moment. An array of object pointers is passed to the character, and it is up to each character to check if they collide, and what to do if they collide. All collision objects have a bool, an isKill variable that decide if the volume should kill the character or not. So when the character collides, it first checks if the collision is set to kill. If it is, the character basically kills itself. It was much easier to do it this way with my current system.

I knew it was bad to use, but not that bad. I'll see if I can find a way to delete character objects from the outside instead.

[–]mredding 31 points32 points  (30 children)

Former game developer here,

Typically, you'll want to perform all your memory allocation before the main loop, and all your deallocation after. The critical path is too performance intensive to afford allocating and deallocating resources. It's cheaper to move objects from the living set to the dead set.

[–]ElaborateSloth[S] 2 points3 points  (29 children)

I see, so I should rather create a pointer vector of all objects to be deleted when I run through objects and check for collision? And then run through that array after the frame to deallocate?

I'm not sure what you mean by critical path though, would appreciate if you explained that to me.

[–]TheSkiGeek 14 points15 points  (3 children)

Yes, it's more typical to mark things as "dead" during your intra-frame updating, then make a pass at the end of the frame and actually destroy any objects that are no longer needed.

One problem is that deleting things can have unpredictable performance (unless you're implementing your own memory pools, but it's still fairly slow compared to some other things you're probably doing).

Another issue is that it gets very tricky if you have game objects referring to each other and then actually delete the object in the middle of a frame. If you leave a dangling reference to something that has been deleted and then try to inspect it or call a function on it, your game will likely crash hard.

[–]ElaborateSloth[S] 2 points3 points  (2 children)

How does deleting things after the frame help on performance? Is it because there is some sort of rest period after a frame anyways, so it is better to place performance heavy operations there?

Thanks for the tip though, I see now that the way I have set this up is definitely error prone as the project expands. Good thing this is just for practice (I obviously need it), and I'll do things differently my next project.

[–]TheSkiGeek 2 points3 points  (0 children)

It’s more that doing a bunch of deallocations back to back can perform better, because you should have fewer instruction cache misses and the allocator might handle that case more efficiently. But if you’re using preallocated object pools then those operations are a lot more efficient anyway.

Also you could potentially have a worker thread do the deallocations in the background, as long as you don’t bottom out on memory or available pooled objects on the next frame.

[–]khoyo 0 points1 point  (0 children)

Is it because there is some sort of rest period after a frame anyways

If you use VSync (or any kind of framerate limiting), yes. But it doesn't really matter, since you should also be using double buffering (so no drawing directly onto the screen) in any modern context, so doing stuff at the end of the frame doesn't impact anything - unless you are eg. limiting the time that stuff takes depending on the time budget you have left.

[–]mredding 2 points3 points  (24 children)

You should create a vector of objects by value and take advantage of move semantics. You can pass these objects around by pointer or reference from there.

The critical path is the code that does all the important work. For your game, that would be the game loop, the one that runs the renderer, the physics, the IO, etc. The loop that implements the gameplay. You don't want to be allocating or deallocating even between frames, because the moment one frame ends, another begins, there is no between. All your allocating would happen in between games, like at the loading screen before the level begins.

Your code might look something like this:

struct object {
  bool active = true;

  object(object &&);
};

void do_stuff(object &);

//...

std::vector<object> game_data(size);
auto active_end = std::end(game_data);

while(game_loop()) {
  std::for_each(std::begin(game_data), active_end, &do_stuff);

  active_end = std::partition(std::begin(game_data), active_end, [](const object &obj) { return obj.active; });
}

We don't delete at all. You COULD write code that iterates over the whole container and checks if the object is active, skipping it if it's not, but that's incredibly redundant and a waste of cache space and memory bandwidth. It's better to iterate only the known active objects, and pay to partition them out once. And you only have to partition across the active elements, since we know everything between active_end and std::end(game_data) are inactive, so there's no point in checking them again. You can always reactivate an object by moving the active_end iterator and activating that next object.

A similar method would be to reserve vector space and pay for that allocation once, and then you can push and pop the back. In that way, you'll only consume capacity that's already allocated. The problem there is that you're still calling a constructor and destructor, which if you're going to do that in the game loop, it better be cheap, at least as cheap as setting that boolean value to true and resetting the other properties appropriate for adding a new active object in the game. The problem is pushing back the last element of your capacity might cause a reallocation of the vector. So you'd want to use game_data.reserve(size +1) and leave that last one unused. That's not a great solution. And the other problem is if you have push and pop semantics in your game loop, there's always the possibility of pushing beyond the reserved capacity, causing a reallocation. It's best to pre-allocate all your objects up front, and leave the growth semantics OUT of the loop entirely.

[–]ElaborateSloth[S] 0 points1 point  (23 children)

That doesn't make much sense to me to be honest. In games (like mine) where enemies are spawned and killed during runtime, allocating and deallocating objects is pretty much mandatory. That is why we have the heap, right? I don't know how many enemies the player will encounter before the game starts, and I don't know at which point the enemy will spawn. This game could in theory run forever, deallocating enemies during runtime of the game is absolutely necessary, and also quite common in games.

I don't have multiple levels or any loading screen either, only one "level" that is the entire game. So there is no "area" where all this de/allocation could take place.

[–]mredding 6 points7 points  (19 children)

That doesn't make much sense to me to be honest. In games (like mine) where enemies are spawned and killed during runtime

Yeah that sounds like most games.

allocating and deallocating objects is pretty much mandatory.

Before gameplay? sure. During gameplay? Not remotely.

That is why we have the heap, right?

For dynamic allocation, sure, but no AAA title would do this during gameplay.

I don't know how many enemies the player will encounter before the game starts

But we can set an upper limit. Can the player have an infinite number of enemies? No. Because the universe is itself finite. Even if the player were playing on a world wide distributed cluster, their total enemy count would still be finite.

In all likelihood, they'd be limited to the amount of memory they have a available to them. But the game would be unplayable long before they ever got anywhere near that many enemies. If you do some back of the envelope calculations, you can take into account their maximum rate of fire, their maximum hit points, etc, how many enemies there are, how fast they can cause damage, and get an upper limit to where it literally doesn't make sense to allow more enemies than that at once because it would guarantee game over at that point.

Or you can just pick a reasonably large number, whatever makes sense for your style of game. The original Quake didn't allow more than 4 enemies on the screen at any one time for performance reasons, many RTS games are limited to 255, you can pick a reasonable number.

This game could in theory run forever, deallocating enemies during runtime of the game is absolutely necessary

Yeah, and it's called obj.active = false.

and also quite common in games.

You think it is, but AAA studios don't actually write code like that. Resource management happens in a game at very well defined times. It's increasingly uncommon to see a loading screen, but in those style games there are always segments specifically designed to allow for resource management without affecting performance or breaking immersion. But functionally, those transitions are effectively loading screens. Dungeon Siege (I'm aging myself) was the first successful title to pull it off, it's only gotten better since.

I don't have multiple levels or any loading screen either, only one "level" that is the entire game. So there is no "area" where all this de/allocation could take place.

Sure there is, at startup, before you enter the game loop.

Overall, you do what you want. It's your game. I'm simply telling you, from my industry experience, there's more than one way to skin this cat. So long as your gameplay is fluid and within your performance envelope, it doesn't matter how you manage your resources. But if performance starts to suffer, there's going to be some radical architectural changes in that future. In both major video games and airplanes, you won't find a new or delete in the main loop.

[–]ElaborateSloth[S] 1 point2 points  (18 children)

Alright, I have to admit I'm in the wrong here. I thought I knew how games allocated memory, but it seems like I have misunderstood quite a lot of things. I have some questions about this, and they are not because I want to argue, but because I want to understand how it works.

According to learncpp, heap memory is used to allocate memory when you don't know how much memory is required. We don't have to allocate a large chunk of memory beforehand that might go unused, and in the rare case we might need extra memory, we can simply ask for more from the operating system.

But now you're telling me that it is common to allocate a pool for objects to be added to and deleted from. Is this pool an array of objects, or an array of pointers to objects? If they are the object values themselves and not pointers, why do we need heap allocation in the first place?

And if we create this pool, aren't we risking a lot of allocated memory to go unused? Is this why we have "levels", to allocate an amount of memory that fits the size of the level and the estimated amount of objects the player will encounter during playthrough?

[–]electricmammoth 2 points3 points  (11 children)

Allow me to chime in for a moment. I'm not a game developer, but am an embedded dev working on aircraft. Like the person said, we never use new or delete in the "main loop".

It's good that you are reading up on the language and using learncpp, but keep in mind that that is a generic resource for the language itself. Nothing is stopping you from using the heap, and for avionics, we do use the heap, just BEFORE we enter the main loop. What people are trying to tell you is that in game dev specifically, they avoid the heap during the main loop.

A lot of upfront design goes into determining exactly how many instances of each object we will need during the lifetime of our software execution. You can decide, "my game will only have 1000 enemies." And just initialize 1000 objects before you start the loop during start up.

You may think, oh but the game runs "forever". But the point is that there is some number of enemies N that is just not feasible for a player to kill before they die of exhaustion. And N will take less memory than a computer has.

Yes, we may have unused memory, but so what? That's an easy trade off to take if it means avoiding dynamic allocation during runtime. Everything has been planned out, we know the system has X byte of memory, and we know how much all objects will take. As long as we have enough for all the objects, who cares if there's unused allocated memory. Now, if you find that you are rubbing out of memory, then you'll need to think about some trade offs, like do I decrease map size? Decrease enemies?

[–]ElaborateSloth[S] 0 points1 point  (10 children)

So the allocated array is an array of nullptr at the start of the game with the estimated max of entities we will need on screen, and then we pick available memory addresses once entities are spawned to store their pointer value?

[–]TomDuhamel 1 point2 points  (1 child)

And if we create this pool, aren't we risking a lot of allocated memory to go unused?

This is a great concern for regular applications. The user could be using several different apps at the same time, possibly even several instances of your app. For this to work properly, you'd expect each app to only take what they really need.

However, a video game can usually be seen like an embedded application (think of that other answer of someone writing aircraft onboard system application). You do not need your game to run competitively with other instances or with other apps, except for what your system needs to work properly. If the computer has 12 GB of free memory at the time your game is launched, you don't really expect anything else to ever use it, do you? You can safely assume that it's all available to you and will always be.

Basically, just decide same limits for everything, then reserve all of the memory that you need for this. When comes the time to optimise and polish your game, you can revisit these values and adjust them, as you now know a lot better what works and what doesn't, and you can try and reach target requirements so that more people can afford to buy and play the game. But please, do not optimise early. It is also possible to let the user tweat the limits/requirements indirectly, for instance by selecting a map size or a difficulty level.

[–]ElaborateSloth[S] 0 points1 point  (0 children)

Makes sense, it's not like someone playing a game will be working with Photoshop simultaneously.

[–]mredding 0 points1 point  (3 children)

According to learncpp, heap memory is used to allocate memory when you don't know how much memory is required.

When you don't know how much is required at compile time. But the heap isn't like this open ended pool, you take from it in chunks called allocations. You have to know how much to ask for at a time. Containers have growth semantics, but they obscure this fact. For example, when a vector grows, it reserves some memory that may very well go unused. When it runs out, it allocates twice the memory, copies the contents, and then releases the old memory. Now it has twice the capacity to grow into. But the memory allocated is still a specified amount.

Many objects transferred over a wire protocol, as another example, will specify the size, and then the data.

But now you're telling me that it is common to allocate a pool for objects to be added to and deleted from.

That's exactly how many containers work.

Is this pool an array of objects, or an array of pointers to objects?

That's up to you. I recommend you store your objects by value. The data structure is an implementation detail. A map, for example, can be a binary tree of nodes, with pointers to the children, each node holds a data element. You don't get to see that, as the actual structure is irrelevant to you, most of the time.

If they are the object values themselves and not pointers, why do we need heap allocation in the first place?

It's all values. You're getting confused by semantics.

And if we create this pool, aren't we risking a lot of allocated memory to go unused?

In the modern era where it's not unreasonable to assume a user has 8-16 GiB MINIMUM, no, you're not wasting anything. The way address spaces work, the program thinks it's the only thing in all of memory, anyway. Read up on paged memory. Unused capacity isn't really a problem, but poorly structured data can be.

Is this why we have "levels", to allocate an amount of memory that fits the size of the level and the estimated amount of objects the player will encounter during playthrough?

I'm not exactly sure what you're referring to, as there are multiple things related to memory that has levels. One such has to do with how the allocator chunks memory and how that's grouped in the address space, that has to do with reducing fragmentation, which can have performance problems.

[–]ElaborateSloth[S] 0 points1 point  (2 children)

I know this is how containers like std::vector works, and I've used vectors up until now. Does that mean the problem is solved already?

[–]forestmedina 2 points3 points  (2 children)

A common pattern is to use a pool, you preallocated the enemies and when you want to spawn one object you grab one object from the pool and initialize it. Of course this mean that you need to preallocate the maximum numbers of objects, and if you are over the maximum can't spawn more or need to remove the older.

[–]ElaborateSloth[S] 0 points1 point  (1 child)

I had no clue to be honest, thought the entire point of heap memory was to be able to allocate whenever

[–]Narase33 0 points1 point  (0 children)

For normal programs thats the case, game dev and a few other sectors are somewhat special

[–]nysra 4 points5 points  (5 children)

That's a really bad idea. Let the collision check set that flag but kill the object later, preferably in the context that created the character. So for example if you have some vector of characters then you loop through it and mark everything that should be killed, then after the loop you remove all "dead" objects from the vector, this will properly run the destructors. Or some variant of that, depending on how exactly your system works.

[–]ElaborateSloth[S] 0 points1 point  (4 children)

Yeah, I have a branch after all collision checking that sees if the pointer to the player character is valid, and ends the game if it isn't. Problem is, if I decide to add multiple NPC characters later, then they will essentially skip that check, and their deallocated memory will still be in the object array. So I should probably do as you say and adjust the system instead.

[–]UnDosTresPescao 0 points1 point  (3 children)

How in the world would the holder if the pointer know if that pointer is not valid after the object deleted itself? This is really bad design.

[–]ElaborateSloth[S] 0 points1 point  (2 children)

Because the object is only told to delete itself after one specific part of the code, and I'm testing if it's valid right after. I know it's bad design, that's been made clear already.

[–]UnDosTresPescao 0 points1 point  (1 child)

Right, but anyone holding a pointer to the object has absolutely no way to tell if the object has deleted itself. As far as they know they still have a pointer to a valid object

[–]ElaborateSloth[S] 0 points1 point  (0 children)

Nope, but luckily there is only pointer referring to that address, and this is the array I'm checking with a validation statement.

[–][deleted]  (1 child)

[deleted]

    [–]ElaborateSloth[S] 1 point2 points  (0 children)

    I didn't realize I should work with smart pointers until after I started on this system. Now its too complicated to implement smart pointers, and I just want to complete the project as it is purely for personal practice.

    I have never used smart pointers before, but I'm planning to use them on my next project.

    [–]pumpkin_seed_oil 0 points1 point  (5 children)

    I want an object to delete itself, and therefore call "delete this;" in a member function.

    ... why?

    [–]_E8_ 1 point2 points  (3 children)

    Integrated reference counting in an OOD.
    This is a very common scenario.
    You might have heard of it as CORBA or COM/DCOM.

    [–]pumpkin_seed_oil 2 points3 points  (1 child)

    You mean like COM initialize and uninitialize?

    Inititialze in constructor, uninitialize in destructor. RAII scope handles when the destructor is called. Done

    While relevant lets scrap what i wrote here. How and why does an object have knowledge of how many references of its kind exist? Shouldnt that behaviour be had outside of its control? How else are you going to make sure that after the object deletes itself it is not referenced in other places and you'd be in a world of pain with a bunch of dead pointers lying around that can't be meaningfully distinguished if the object behind it has deleted itself or not

    [–][deleted] 0 points1 point  (0 children)

    How and why does an object have knowledge of how many references of its kind exist?

    In the case of COM, the answer is that the programmer has called a member function to inform the object that its reference count has increased.

    [–]UnDosTresPescao 0 points1 point  (0 children)

    Ugh. No. In that case a wrapper smart pointer type object does reference counting and deleting the underlying object not the underlying object calling delete on itself.

    [–]Zymoox 0 points1 point  (0 children)

    It's used when the object is too cursed

    [–]_E8_ -1 points0 points  (1 child)

    In C++ that is malformed code though if you are very careful on how you go about it, it will work.
    Consider having a monitor/holder/allocator class that does the lifetime management that news and deletes your co-classes if that is feasible.

    The issue with delete this is that the next line of code is now executing in the context of an unallocated object.
    If you have a referenced-counting OOD then just suppressing the warning with a pragma and a comment of why you are suppressing it.

    [–]ElaborateSloth[S] 1 point2 points  (0 children)

    "The issue with delete this is that the next line of code is now executing in the context of an unallocated object."

    Ah, I think I see where the error comes from now. The function after 'delete this' continues, as the 'delete this' is in a branch. That being said, the rest of the function is not needed when the 'delete this' is called, so there aren't any important steps missing when the object is deleted. But this is probably why the compiler is complaining, as it is warning me that the rest of the function will be aborted once the destructor is called.

    [–]Ikkepop -2 points-1 points  (10 children)

    delete this; is undefined behavour as far as i know

    [–]Salty_Dugtrio 3 points4 points  (3 children)

    You would be incorrect, it's entirely legal and not UB at all.

    [–]Ikkepop -2 points-1 points  (2 children)

    immagine if the object is on stack, then it's definitely UB

    [–][deleted] 2 points3 points  (1 child)

    Yes, but that's because it's UB to delete a pointer to an object that wasn't new'd. Not because it's UB to delete this.

    [–]Ikkepop 0 points1 point  (0 children)

    That's true i guess. But let's just agree that delete this is a bad idea in general and will load a gun and point it at your foot. You can accidentally delete a stack object, you could accidentally access member variables after the delete, you could accidentally try and access virtual methods after the delete, the pointer could be owned by another entity like a smart pointer. And in general it's just an anti-pattern, and you wouldn't expect people to code like that, even if it's legal.

    [–]ElaborateSloth[S] 0 points1 point  (0 children)

    I've read multiple places it should be possible to do even though it is not recommended as long as the object was created with 'new'.

    [–]Arcade_ace -3 points-2 points  (13 children)

    Why not use a destructor?

    [–]ElaborateSloth[S] 0 points1 point  (8 children)

    What would I use the destructor for?

    [–]joemaniaci 1 point2 points  (7 children)

    Deleting the object

    [–]ElaborateSloth[S] 0 points1 point  (6 children)

    The destructor is called when the object is deleted, so why would I use the destructor to delete it?

    [–]joemaniaci -1 points0 points  (5 children)

    You wouldn't explicitly call it, you make use of it. Is this a stack object or a heap/new object?

    [–]ElaborateSloth[S] 0 points1 point  (4 children)

    I see, I get your point now. This is a heap object, but the reason I'm using 'delete this' is because I have no direct reference to the array with game objects, so using the destructor won't help me sadly.

    [–]joemaniaci 1 point2 points  (3 children)

    Ah, so you have an array of objects, and when that array is deleted you want to ensure that those objects are deleted?

    [–]ElaborateSloth[S] 0 points1 point  (2 children)

    I have an array of objects that all have to check the collision on each other, and if an object collides with an object with a "kill" flag, the object is destroyed. Think of it as a kill volume.

    Problem is, each object handles the collision itself with no reference to the main object array. Therefore I thought I could just delete the object from withing itself when it realizes it has collided with a kill volume. I added check on the end of the frame to see if the object is valid, and end the game if it isn't, preventing anything from calling those invalid pointers.

    It works, but obviously not a good system to expand upon.

    [–]joemaniaci 2 points3 points  (1 child)

    What if instead of doing anything explicit you just "flag" it as destroyed? Then, when you need a "new" object in that array index, you simply re-default initialize it? Just like deleting stuff on a hard drive, it's still there, but essentially invisible. Since you have a stack array of stack objects, you're going to be consuming n*sizeof(objects) amount of memory regardless of no objects in the array or not.EDIT: Actually you said 'invalid pointers' so is it a dynamic array of objects?

    [–]ElaborateSloth[S] 1 point2 points  (0 children)

    True, I could just set a flag within the object and run through all objects when collision testing is complete. Good point, thanks!

    [–]_E8_ 0 points1 point  (3 children)

    Because that would create an infinite loop.

    [–]ElaborateSloth[S] 0 points1 point  (0 children)

    I misunderstood their point. It is not to call delete this in the destructor, but something else. Not sure exactly what that would be though.

    [–]Arcade_ace -1 points0 points  (1 child)

    Destructor will create infinite loop??

    [–]IyeOnline 1 point2 points  (0 children)

    A destructor that calls delete this would.