Unittests pass, and then an invalid memory operation happens after?

[Liam McGillivray]

On Friday, 29 March 2024 at 01:18:22 UTC, H. S. Teoh wrote:
> Take a look at the docs for core.memory.GC.  There *is* a 
> method GC.free that you can use to manually deallocate 
> GC-allocated memory if you so wish.  Keep in mind, though, that 
> manually managing memory in this way invites memory-related 
> errors. That's not something I recommend unless you're adamant 
> about doing everything the manual way.

Was this function removed from the library? I don't see it in 
[the document](https://dlang.org/phobos/core_memory.html).

How is `GC.free` different from `destroy`?

I might take a look at it, but I'm not adamant about doing 
everything the manual way, so I probably won't use it very soon.

> I think you're conflating two separate concepts, and it would 
> help to distinguish between them.  There's the lifetime of a 
> memory-allocated object, which is how long an object remains in 
> the part of the heap that's allocated to it.  It begins when 
> you allocate the object with `new`, and ends with the GC finds 
> that it's no longer referenced and collects it.

No. I understand that when an object disappears from memory might 
happen after it disappears from the game. I'll explain later in 
this post why I wanted to remove the Unit object from memory when 
the unit dies.

> There's a different lifetime that you appear to be talking 
> about: the logical lifetime of an in-game object (not to be 
> confused with an "object" in the OO sense, though the two may 
> overlap).  The (game) object gets created (comes into existence 
> in the simulated game world) at a certain point in game time, 
> until something in the game simulation decides that it should 
> no longer exist (it got destroyed, replaced with another 
> object, whatever). At that point, it should be removed from the 
> game simulation, and that's probably also what you have in mind 
> when you mentioned your "die" function.

Yes; exactly. This was your hint that I'm not confusing these two 
things. Whether or not the unit object gets deleted, I need a 
function to remove it from the game on death.

The `die` function if I want the object to be destroyed on death:
```
void die() {
         if (this.map !is null) this.map.removeUnit(this);
         if (this.faction !is null) this.faction.removeUnit(this);
         if (this.currentTile !is null) this.currentTile.occupant 
= null;
         destroy(this);
     }
```
The `die` function without object destruction:
```
void die() {
         if (this.map !is null) this.map.removeUnit(this);
         if (this.faction !is null) this.faction.removeUnit(this);
         if (this.currentTile !is null) this.currentTile.occupant 
= null;
     }
```

They're the same, except that the latter doesn't call `destroy`. 
The other 3 lines are required to remove references to the 
object, effectively removing it from the game.

With the latter version, I suppose that the garbage collector 
should eventually clean up the "dead" unit object now that there 
are no references to it. However, I can see this leading to bugs 
if there was another reference to the unit which I forgot to 
remove. One benefit I see in destroying the object when it's no 
longer needed is that an error will happen if any remaining 
reference to the object gets accessed, rather than it leading to 
unexpected behaviour.

However I've thought about having it destroy the unit object at 
the end of the turn rather than immediately. Another option, if I 
don't want this benefit for debugging but still want fewer 
deallocations in the end result, would be to set that last line 
to `version (debug) destroy (this)`.

Anyway, I made [a 
commit](https://github.com/LiamM32/Open_Emblem/commit/64109e556a09ecce73b1018a9e651744a5e8fcd9) a few days ago that solves the unittest error. I found that explicitly destroying every `Map` object at the end of each unittest that uses it resolved the error. Despite this resolving the error, I decided to also move those lines from the `Unit` destructor to the new `die` function. I currently have it call `destroy` on itself at the end of this new function for the reasons described, but I suppose this line can be removed if I want to.

> And here's the important point: the two *do not need to 
> coincide*. Here's a concrete example of what I mean. Suppose in 
> your game there's some in-game mechanic that's creating N 
> objects per M turns, and another mechanic that's destroying 
> some of these objects every L turns.  If you map these 
> creations/destructions with the object lifetime, you're looking 
> at a *lot* of memory allocations and deallocations throughout 
> the course of your game.  Memory allocations and deallocations 
> can be costly; this can become a problem if you're talking 
> about a large number of objects, or if they're being 
> created/destroyed very rapidly (e.g., they are fragments flying 
> out from explosions).  Since most of these objects are 
> identical in type, one way of optimizing the code is to 
> preallocate them: before starting your main loop, say you 
> allocate an array of say, 100 objects. Or 1000 or 10000, 
> however many you anticipate you'll need. These objects aren't 
> actually in the game world yet; you're merely reserving the 
> memory for them beforehand. Mark each of them with a 
> "live"-ness flag that indicates whether or not they're actually 
> in the game.  Then during your main loop, whenever you need to 
> create a new object of that type, don't allocate memory for it; 
> just find a non-live object in this array, set its fields to 
> the right values, and mark it "live".  Now it's a object in the 
> game.  When the object is destroyed in-game, don't deallocate 
> it; instead, just set its "live" flag to false.  Now you can 
> blast through hundreds and thousands of these objects without 
> incurring the cost of allocating and deallocating them every 
> time.  You also save on GC cost (there's nothing for the GC to 
> collect, so it doesn't need to run at all).

Well, I don't have any explosions or anything fancy like that. 
Either way, I think all this will run very very fast. The one 
idea I have that might change this is if I have the enemy AI look 
multiple turns ahead by cloning all game objects in order to 
simulate multiple future outcomes.

When you mention a "flag" to indicate whether they are "live", do 
you mean like a boolean member variable for the `Unit` object? 
Like `bool alive;`?

> My advice remains the same: just let the GC do its job. Don't 
> "optimize" prematurely.  Use a profiler to test your program 
> and identify its real bottlenecks before embarking on these 
> often needlessly complicated premature optimizations that may 
> turn out to be completely unnecessary.

Alright. I suppose that some of the optimization decisions I have 
made so far may have resulted in less readable code for little 
performance benefit. Now I'm trying to worry less about 
optimization. Everything has been very fast so far.

I haven't used a profiler yet, but I may like to try it.

> If you're conscious of performance, however, I'd say avoid 
> references where you can.  Since maps presumably will always 
> exist while the game is going on, why bother with references at 
> all?  Just use a struct to store the coordinates of the tile, 
> and look it up in the map.  Or if you need to distinguish 
> between tiles belonging to multiple simultaneous maps, then 
> store a reference to the parent map along with the coordinates, 
> then you'll be able to find the right Tile easily. This way 
> your maps can just store an array of Tile structs (single 
> allocation), instead of an array of Tile objects (M*N 
> allocations for an M×N map).

It's unlikely that I will have multiple maps running 
simultaneously, unless if I do the AI thing mentioned above. I've 
had a dilemma of passing around references to the tile object vs 
passing around the coordinates, as is mentioned in an earlier 
thread that I started. In what way do references slow down 
performance? Would passing around a pair of coordinates to 
functions be better?