Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
Sign in to follow this  
Quasar

C++ custom arenas and explicit deallocation

Recommended Posts

I realized that when zone blocks are implicitly freed, the corresponding objects' destructors are naturally not executed, since the blocks are just Z_Free'd and not removed via operator delete. This only currently works because all such objects have trivial destructors.

This has some unfortunate consequences for future development in Eternity, as well as raises some other closely related problems into the spotlight.

We wanted to make a ZoneObject class, which could act as the base class for objects interested in supporting allocation on the zone heap. However, since operator new can't effectively set the tag of the allocation without hard-coding it, and we do not want each class type or hierarchy tied only to a single tag, the idea was to perhaps pass the tag into certain base class constructors as an optional parameter.

The problems with this are twofold: 1. if the object is *not* allocated on the zone heap, then trying to Z_ChangeTag it in the constructor will cause an illegal operation, and 2. it isn't necessarily guaranteed that the pointer returned by operator new is equal to the most-ancestral base class. This is generally the case on 99% of C++ compilers, but the standard doesn't promise the base-most class is first in memory.

In addition, if non-trivial destructors are allowed for zone allocated objects, then any memory allocated inside the class's constructor and released explicitly by its destructor has to be allocated under a non-volatile tag (ie. PU_STATIC, but not tags such as PU_LEVEL or PU_CACHE), because during a Z_FreeTags operation this would cause order-of-destruction dependencies which could only be resolved through introduction of a full garbage collector >_>

I suppose my point here is just to solicit ideas on what the least of the evils might be for a tolerable solution to these issues.

Share this post


Link to post

Well, my advice to this issue is to not let it become an issue. The zone heap with its option to 'forget' all allocations doesn't really work with C++ as you noticed.

I think the better approach would be to handle object storage in a way that makes deletion a simple and straightforward task without having to resort to just deleting the heap info. Like it or not, the zone heap may be convenient for this but it sure leads to sloppy programming. If you don't let it get this far object deletion won't become an issue.

Or implement a garbage collector like ZDoom does. ;)

Share this post


Link to post

If you consider that the only time when you want to be able to forget about allocations is in code that should be garbage collected anyhow (e.g., in an in-game scripting system with dynamic object creation) then there really isn't many plus sides to retaining the Zone heap in a modern DOOM engine.

Share this post


Link to post

Arenas aren't sloppy programming, so I disagree with that. They're used all the time, and they're a good way to *stop* having to worry about individually managing every single allocation - which is precisely what leads to memory leaks in the first place.

By having a tag that releases all resources of a certain type at a certain time, you are sure to catch them all. I don't consider this sloppy as opposed to this:

   Z_Free(ptr1);
   Z_Free(ptr2);
   Z_Free(ptr3);
   Z_Free(ptr4);
   ....
   Z_Free(ptr25);
We used to have a segment a bit like that, in the renderer. It was needed when new wads are loaded, so that everything created during R_Init could be trashed. Instead now this is handled by a PU_RENDERER tag. All renderer allocations are done under PU_RENDERER, and thus if R_Init needs to run again, it can just Z_FreeTags and be assured that everything is released. This is way easier than forgetting to put a Z_Free in a function somewhere.

Also I should probably note that EE's "zone" heap is not a real zone heap, and has not been for a long time now (a couple years at least). It is a thin wrapper on the C heap that provides these domain-specific allocation lifetimes. One such lifetime I have added and we now *extensively* rely upon is the PU_AUTO tag - PU_AUTO blocks are garbage collected at the end of the main loop and are used to implement a portable alloca.

So I was really hoping for more advice than "trash z_native.cpp" because I do think there might be solutions to some of these issues that I simply haven't envisioned :/

Share this post


Link to post
Graf Zahl said:

Or implement a garbage collector like ZDoom does. ;)


Didn't ZDoom recently got a memory arena system as well? Or is that a different concept for arena?

Share this post


Link to post

I don't think either Graf or I was suggesting that arenas aren't useful. What I was saying is that the Zone (as it is in DOOM) is no longer useful as a general purpose memory manager.

If you have already redesigned the Zone as you say then what I suggest is: duplicate the "Zone" class hierarchy for each problem domain and then specialize for each.

Share this post


Link to post
Gez said:

Didn't ZDoom recently got a memory arena system as well? Or is that a different concept for arena?



It's different as each arena manages its own memory instead of using tags. And it can't handle classes with destructors.

However, in ZDoom it's only used for specific purposes where a small piece of code needs to do a large number of allocations that all need to be freed at once. For such uses it's fine.

The original zone heap's PU_LEVEL purging on the other hand is what I call 'sloppy programming'. No thought was invested into self-cleaning data structures because it just dumped everything at the end of a level.

(As for the aforementioned PU_AUTO... all I can say is 'ouch'!)

Share this post


Link to post
Graf Zahl said:

(As for the aforementioned PU_AUTO... all I can say is 'ouch'!)

So you don't see the value in disposable buffers? Granted with C++ it might would be more "usual" to use an "auto" (as in the never-used keyword for stack storage, since it's implicit for function-locals) object that is destroyed when it falls out of scope, but we needed a similar mechanism long before we converted to C++ - freeing a string, for example, at every return statement is very error-prone and in some cases difficult. Either way it accomplishes mostly the same thing. It's an allocation you allow to be done and then have freed implicitly later on.

The one advantage PU_AUTO does have over auto-storage with object destructors is that it can be used to create PODs such as raw char buffers or a bool array for temporary marking in an algorithm, etc., without having to drum up a full class abstraction where it's not really needed.

Share this post


Link to post
Quasar said:

So you don't see the value in disposable buffers?



What bothers me here is that these buffers accumulate in an uncontrolled fashion before they eventually get deleted. I certainly can see the attraction in them but to me they hardly qualify as good programming.

Share this post


Link to post
Graf Zahl said:

What bothers me here is that these buffers accumulate in an uncontrolled fashion before they eventually get deleted. I certainly can see the attraction in them but to me they hardly qualify as good programming.

You can have the same problem with general garbage collection; it comes up in Java quite a bit in fact, in complicated frameworks - the VM starts acting like it's out of memory because there are too many actually dead objects that aren't marked as collectable yet. IIRC they had serious trouble with it while developing the Swing UI.

Anyway, these are largely used during startup anyway. Out of code that runs during the main loop, only some console command processing uses them otherwise.

Share this post


Link to post
Quasar said:

and we do not want each class type or hierarchy tied only to a single tag


Why? When are you going to need a PU_STATIC vertex or actor? Really the only things that shouldn't be kept between maps is the map data, actors and maybe the music(since people love huge MP3s). There's no real point is freeing each levels flats/sprites since they're just be used again. The patches used only to build composite textures can probably be free separately.

Share this post


Link to post
Scet said:

Why? When are you going to need a PU_STATIC vertex or actor? Really the only things that shouldn't be kept between maps is the map data, actors and maybe the music(since people love huge MP3s). There's no real point is freeing each levels flats/sprites since they're just be used again. The patches used only to build composite textures can probably be free separately.

We don't. But it would be nice to, for example, have PU_STATIC and PU_LEVEL variants of the MetaTable. In fact this will probably become an absolute necessity later on. PU_AUTO could be used by qstrings, although I'd probably do better at this point to make them use a destructor instead.

PU_CACHE is used for a lot more things than just flats and sprites, btw. I agree PU_CACHE is not really very critical any more - in fact, EE only currently ever frees PU_CACHE blocks if an *operating system* out-of-memory error occurs. This means you generally end up with the entire IWAD loaded into memory eventually, which makes the game run faster.

SoM and I have agreed that we can "program by contract" as far as not mixing volatile tags and explicit resource management goes. However, I still have not found a satisfactory mechanism for allowing the zone system to execute destructors on anything.

Share this post


Link to post

I still stand by my opinion that the best solution is to define proper lifetime cycles for all data and explicitly free it if no longer needed.

Like everything convenient it eventually comes back biting you in the back and making your life harder than necessary.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  
×