Jump to content

Ultra Software Company Blog

  • entries
    185
  • comments
    1,247
  • views
    563,974

Contributors to this blog

Rewerking the Asset Class -or- Back that Asset Up


Josh

6,318 views

 Share

Since you guys are my boss, in a way, I wanted to report to you what I spent the last few days doing.

 

Early on in the development of Leadwerks 3, I had to figure out a way to handle the management of assets that are shared across many objects. Materials, textures, shaders, and a few other things can be used by many different objects, but they consume resources, so its important for the engine to clean them up when they are no longer needed.

 

I decided to implement an "Asset" and "AssetReference" class. The AssetReference object contains the actual data, and the Asset object is just a handle to the AssetReference. ("AssetBase" probably would have been a more appropriate name). Each AssetReference can have multiple instances (Assets). When a new Asset is created, the AssetReference's instance count is incremented. When an Asset is deleted, its AssetReference's instance counter is decremented. When the AssetReference instance counter reaches zero, the AssetReference object itself is deleted.

 

Each different kind of Asset had a class that extended each of these classes. For example, there was the Texture and TextureReference classes. The real OpenGL texture handle was stored in the TextureReference class.

 

Normal usage would involve deleting extra instances of the object, as follows:

Material* material = Material::Create();
Texture* texture = Texture::Load("myimage.tex");
material->SetTexture(texture);// This will create a new instance of the texture
delete texture;

 

This isn't a bad setup, but it creates a lot of extra classes. Remember, each of the AssetReference classes actually get extended for the graphics module, so we have the following classes:

Asset
Texture
AssetReference
TextureReference
OpenGL2TextureReference

OpenGLES2TextureReference

 

The "Texture" class is the only class the programmer (you) needs to see or deal with, though.

 

Anyways, this struck me as a bad design. Several months ago I decided I would redo this before the final release. I got rid of all the weird "Reference" classes and instead used reference counting built into the base Object class, from which these are all derived.

 

The usage for you, the end user, isn't drastically different:

 

Material* material = Material::Create();
Texture* texture = Texture::Load("myimage.tex");
material->SetTexture(texture);
texture->Release();// Decrements the reference counter and deletes the object when it reaches zero

 

In the Material's destructor, it will actually decrement each of its texture objects, so they will automatically get cleaned up if they are no longer needed. In the example below, the texture will be deleted from memory at the end of the code:

Material* material = Material::Create();
Texture* texture = Texture::Load("myimage.tex");
material->SetTexture(texture);
texture->Release();
material->Release();// texture's ref count will be decremented to zero and it will be deleted

 

If you want to make an extra "instance" (not really) of the Asset, just increment the reference counter:

 

Material* material = Material::Create();
Texture* texture = Texture::Load("myimage.tex");
texture->IncRefCount();// increments ref count from 1 to 2
Texture* tex2 = texture;
texture->Release();// decrements ref count from 2 to 1

 

This makes the code smaller, gets rid of a lot of classes, and I think it will be easier to explain to game studios when I am trying to sell them a source code license.

 

Naturally any time you make systemic changes to the engine there will be some bugs here and there, but I have the engine and editor running, and mistakes are easy to find when I debug the static library.

 

I also learned that operation overloading doesn't work with pointers, which i did not know, but had never had a need to try before. Originally, I was going to use operation overloads for the inc/dec ref count commands:

Texture* tex = Texture::Create();//refcount=1
tex++;//refcount=2
tex--;//refcount=1
tex--;// texture would get deleted here

 

But that just messes with the pointer! So don't do things like that.

 Share

22 Comments


Recommended Comments

looks like COM programming. not a fan of that really. I'd prefer to call delete instead of ->Release() for the simple fact that that's what we do with pointers. Having to call IncRefCount() is even worse. That'll be forgotten by people a large % of the time I'm guessing and cause some nice bugs.

  • Upvote 1
Link to comment

I prefer that every definition, type, function, class and method are tailor made for the engine. That's how professional libraries do it too, and rarely depend on C++'s native functions and methods. In Qt it's actually done a bit clumsy, and it's kinda annoying that you can't use std::string with Qt, but you have to use QString. So, keep your own classes and methods still compatible with STL classes.

Link to comment

I think having to implement basic functionality that should have been thought about when C++ was first designed is a huge weakness of the language, but every language since then has been managed code, so I guess it's the best thing we've got.

Link to comment

Yeah.. this is the way COM works (also see DirectX). Works well. You can also build in some automatically ref-counting in assignment operators, copy constructors and in destructors.

Link to comment

Yeah.. this is the way COM works (also see DirectX). Works well. You can also build in some automatically ref-counting in assignment operators, copy constructors and in destructors.

But not for pointers, right?

Link to comment

I think Ken mentioned in the other post that doing (*var)++ should work. Haven't tried it but seems like it would. Even if that does work I personally think that's messy. If I had the choice I'd rather call the function so it's clear and obvious what's happening.

Link to comment

I agree with Rick. It seems counter-intuitive to me to have to call IncRefCount. After all, why would I? I have a texture object, I want to use it, why should I do anything to it first? Similarly, using Release() instead of delete seems just as counter intuitive. Normal C++ would do exactly as in your first example.

 

I understand you're removing internal classes this way, but for the end user, it's not prettier, not more intuitive, and will actually be a bother in the long run if we have to type this for every object.

 

In the "source purity" aspect, this has the adverse effect of making me think this wasn't designed for C++, but just patched to work with it.

Link to comment

material->SetTexture(texture);// This will create a new instance of the texture

Why would the above line create a new instance? Wouldn't it be

Texture* texture = Texture::Load("myimage.tex");

that actually creates the instance? I can see the SetTexture() line adding to the ref count but not creating a new instance.

 

 

Also, why do we have the static Load() and Create() methods again vs using new? I know there was a reason but can't seem to remember.

Link to comment

The alternative to having a memory management system like this is either memory leaks or invalid pointer deletion crashes.

 

Or I could just make every call to the asset load functions uniquely load the data, and your video memory usage would increase 100 times.

Link to comment

The alternative to having a memory management system like this is either memory leaks or invalid pointer deletion crashes.

That's not a fatality. Yes, with people who do not understand how memory management works and do not call delete, this will happen. If you code properly, it won't. In the same spirit, using your newly proposed system will lead to invalid reference count crashes or objects that stay in memory indefinitely when they shouldn't, because people forget to call "IncRefCount" or "Release" or call them too often.

 

 

Also, why do we have the static Load() and Create() methods again vs using new? I know there was a reason but can't seem to remember.

 

Constructors are not passable through a DLL for other languages, I think this was the reason. Also, in languages that require class wrapping (Java, C#, etc.), using static classes allowed to have 'Entity::Create' instead of 'Engine::CreateEntity', which is a nice syntactic sugar.

Link to comment

You do not need to call IncRefCount(). It's just used internally.

 

Yes, with people who do not understand how memory management works and do not call delete, this will happen.

You would effectively be writing your own engine at that level. If I load a model, for example, and then delete the model, it's reasonable to expect the model's materials, textures, and shaders to also get deleted if they are no longer in use.

 

I looked up COM and it's identical to what I am doing. Maybe I will even make the syntax match it exactly.

Link to comment

Since there is no "new" needed, there should be also no "delete" needed. The worst would be if you mix "new" and "delete" with ".Create()" and ".Release()". But since ".Create()" and ".Release()" can have much better error checks, and it looks also much more intuitive than "new" and "delete".

 

I don't want to make a "new" texture, but just load my old texture from disk. Also I don't want to "delete" my texture from disk, but just unload/release it from memory smile.png

 

After all LE3 is an 3D Engine, not a C++ programming language, and it should have context related commands, not C++ commands. And by using engine commands only, all the C++/C#/Java/Lua/etc... game source code would be indentical. You can use whatever language you like, and it's exactly the same source code! That's what I call cross-language compatibility :)

  • Upvote 2
Link to comment

You do not need to call IncRefCount(). It's just used internally.

 

In you example where you say pass the texture to some other user object, then that makes another reference and we would have to call InRefCount() to be correct wouldn't we?

 

I looked up COM and it's identical to what I am doing. Maybe I will even make the syntax match it exactly.

 

People like the idea of what COM gives them, but I think in general people hate programming in it. Which is why they've created wrappers to it for these other languages like VBA, etc.

 

If I load a model, for example, and then delete the model, it's reasonable to expect the model's materials, textures, and shaders to also get deleted if they are no longer in use.

 

I agree with the above for sure, but it sounded like you had that setup already, but it added some classes to your source and you were trying to keep it "clean" for potential buyers of the source. I would assume you'll have more users than buyers so if it was easier for the user before but added classes to the source, then I would think that's the way you want to go.

 

 

I don't know how the internals of LE3 are setup, but some sort of singleton AssetManagement class that manages the instancing of LE objects seems like a good idea at first glance. A map where the key is the filename+path and multiple calls to Load() or new would check if the object is already loaded and handle the instance count for us. If we pass these around to other objects then it's on us to manage it correctly. Just tossing around ideas. It's tough without seeing the entire picture.

 

On a side note, I think Orge has a good model for C++ engine. I don't like the engine itself but it's not because of how it's coded in C++. Something like the below would make more "sense" from a programmer perspective I think:

 

Model* model = gfxDevice->LoadModel("");

 

Ogre uses the scene object to load/create objects but from what I remember the GFX device is really what needs to control this for LE. Either this way or you could pass in the gfxDevice to the ctors to get the functionality. The nice thing about this is that the management of the objects can be handled in the gfxDevice object and they wouldn't be stored globally. Again, not knowing how LE3 is currently setup, I'm guessing the engine objects are stored globally internally which I don't think they need to be.

 

One might think the downside to this is that you have to keep and pass around this gfxDevice object, but I don't see that as a bad thing. Someone can make it global if they like, or pass it around to their game states.

Link to comment
A map where the key is the filename+path and multiple calls to Load() or new would check if the object is already loaded and handle the instance count for us.

That's exactly what it does.

 

If we pass these around to other objects then it's on us to manage it correctly.
So you'll implement your own ref counting system, and then instead of a standard one built into the engine, we'll have a dozen non-standard approaches, all trying to make up for lacking functionality. :\
Link to comment

So you'll implement your own ref counting system, and then instead of a standard one built into the engine, we'll have a dozen non-standard approaches, all trying to make up for lacking functionality. :\

 

When we pass things to LE methods, LE needs to handle it, like it seems it was before you wanted to make the change. If we pass these to our own methods then it's on us. It's then no different than any other pointer to a class we make and pass it around for actual storage inside other classes.

 

How often do we really pass LE objects to be stored in classes vs having the LE objects directly part of a class or just used directly without storage in a method.

 

The one scenario I ran into was passing an Enemy pointer to my Wizard class and storing it there as it's active target. This proved to be very bad since that Enemy could die, and be deleted, outside of that Wizard class (another wizard might have had the same target and killed it). My design was poor. Passing these pointers to be stored in multiple places seems like a poor design on the developers side if they choose to do that. I think this is just inherant to working with C++ and pointers.

 

A similar issue can happen in LE2 can it not? I can create a TTexture, pass it along to 3 different classes to be stored, then delete it in 1 class, and screw the other 2 classes that were using it.

Link to comment

This makes the problems you listed go away. For example this code will only load the texture once, and at the end it will delete the texture:

 

Texture* texture = Texture::Load("myimage.tex");

material->SetTexture(texture);

material2->SetTexture(texture);

texture->Release();

texture = Texture::Load("myimage.tex");

material3->SetTexture(texture);

texture->Release();

material->Release();

material2->Release();

material3->Release();

 

In your entity pointer target example you could have incremented the reference count, used Release() instead of delete, and you would have been safe. (I renamed the IncRefCount() function to AddRef() to match the COM syntax, which uses an identical system).

 

Your wizard update function might have looked like this:

if (activetarget->GetKey("health")<=0)

{

activetarget->Release();

activetarget = NULL;

}

 

Setting the target might be like this:

void Wizard::SetActiveTarget(Entity* entity)

{

if (activetarget) activetarget->Release();// decrement the ref counter of the old one, if it exists

activetarget = entity;

if (activetarget) activetarget->AddRef();// increment the ref counter of the new one so we can hold onto it

}

Link to comment

The Asset class destructor actually looked like this before my changes:

Asset::~Asset()

{

this->assetreference->instancecount--;

if (this->assetreference->instancecount==0) delete this->assetreference;

}

 

So it was really the same thing, just with a lot of extra classes in an attempt to continue using "delete" when it really isn't appropriate.

Link to comment

I guess my point, and the point of why some people hate COM, is that your first example is a mess. It's a nightmare of following/matching AddRef() and Release() function calls, and that's all in one place. As soon as you spread that out into many different classes and functions it gets even harder. The better option for me was to not pass and store the Texture/Wizard pointer in another class at all, but to use an asset management class and store int ID's instead to avoid pointer nightmare.

 

I would just say adding this design will touch every piece of the way to program in C++ (what about Lua and other languages?) and is a pretty big decision. A good number of people dislike having to manually maintain the ref count. From COM wiki:

 

"To facilitate and promote COM development, Microsoft introduced ATL (Active Template Library) for C++ developers. ATL provides for a higher-level COM development paradigm. It also shields COM client application developers from the need to directly maintain reference counting, by providing smart pointer objects."

 

"Certain languages (e.g. Visual Basic) provide automatic reference counting so that COM object developers need not explicitly maintain any internal reference counter in their source codes."

 

There are reasons why MS created the above 2 things. I think they realized that people hated working with aspects of COM, the ref count being one of them. Having to maintain and match up AddRef() with Release() can be a pain.

  • Upvote 1
Link to comment

Smart pointers really do seem interesting. I wouldn't want to have to manage my ref count either.

 

Think of the Lua scripters, who use this language to avoid the complexity of C++. If they don't want to deal with pointers, they certainly wouldn't want to deal with reference count either.

 

They'd want it to "Just work".

Link to comment

All those solutions like smart pointers and garbage collection usually lead to much worse problems:

http://www.codingwis...or-retards.html

 

If you are afraid of managing your memory manually, you should be terrified of debugging memory leaks when memory management is automated. I've done it, and it's a nightmare.

Link to comment

About that article:

 

1) That guy sounds like a real gem.

 

2) That article basically shows and explains why you shouldn't be trying to do what you are Josh.

 

 

His watcher approach is something I've tried also. In my example, right before an enemy pointer was deleted (delete enemy), it would fire an event that the enemy had. When I originally assigned this enemy as a target to a wizard, that wizard would register to this delete event. So now every object that needed to know when this enemy was killed and got deleted would be notified and can then do what it needed to do with it's pointer to the object it had.

 

In the end I didn't go this route though. I decided to make a singleton class that held all the enemies and each enemy had an int ID > 0. When I assigned the target I just passed the int ID to the wizard. Every time in the wizard class I wanted to use the target I would pass the in ID to the singleton GetObject(id) function to get the pointer to the object IF it wasn't deleted. If it was I just got back NULL and then knew that my target was deleted.

 

I actually want to explore the event method more because I think it's more elegant and doesn't require sort of global objects in the game like a singleton is basically doing.

 

The even way however won't work in your situation because that's more setup to tell the object that shares the pointer that the pointer is no longer valid, meaning it has already been deleted and there isn't anything it can do about it. In your case you don't want to actually delete the pointer until everything is finished referencing it.

Link to comment
Guest
Add a comment...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...