   A Simple Fast Resource Manager using C and STL
  Comments for the article A Simple Fast Resource Manager using C++ and STL
Author BiosAshic Mahtab is a final year student of the Department of Computer Science and Engineering of the Bangladesh University of Engineering and Technology (BUET). He is currently working on his thesis (with Zinat) on the design and development of a plug-in based game engine. His interests include software engineering, game programming and .Net technologies. He is also a freelancer focusing on developing solutions in .Net. He can be reached at aheartattack@gmail.com.Zinat Wali is a final year student of CSE Department of BUET (Bangladesh University of Engineering and Technology). She is currently doing her thesis on the design and development of a plug-in based game engine. She is also interested in .Net technologies and does freelance development. She can be reached at zinat.wali@gmail.com.   ;  Overall a good article. The code seems well implemented and the article is well written, and tackles a common issue, but I feel the design decisions could have been a little more carefully thought out.This design imposes a separate ResourceManager on every type of resource, which quickly multiplies, and is not ideal in a dynamic plugin-based environment. It also requires that every type of resource be derived from the resource class, which may well require a lot of jumping through hoops when integrating 3rd-party components.It also appears that the path, handle, and possibly name don't really belong as attributes of the Resource class, and might be better placed in the ResourceManager.Lastly, and perhaps most importantly, the authors provide an example wherein the Mesh resource knows how to render itself. To my mind at least, this is a very large violation of separation of responsibilities. A resource should merely be a container for a collection of data, which knows how to serialise/de-serialise itself. Ideally it should perform no operations on that data (beyond decompression/reconstruction), and operations such as rendering should be delegated to a Renderer, or the like.   ;Here's my critique:It seems that the source is rather quirky and very reminiscent of Java despite being C++. As such it's not really as simple or as fast as it ought to be; or perhaps the implementation is just a bit too simple.Overall I struggle to find a proper justification for the manager to exist at all.Some points:* There are loops that don't need to be there (see EmptyList).* There are conditions that don't need to be there (see Resource constructor,  checks for negative unsigned integers and checks for m_list == NULL).* There are pointers that shouldn't be pointers (see m_list).* Functions with no clear justification for them (see GetList).* There is no suitable copy constructor for the manager.* A comment refers to the vector copy-trick but performs the trick incorrectly (see EmptyList).* Get member functions return strings by value instead of const-reference.* The Add member function returns -1 on failure yet the return type is unsigned.* The SAFE_DELETE macro shouldn't be a macro, it's used inconsistently and it's inefficient.* Nested conditions that could be combined into one (see GetElement(string, string))* Searching by string and insertion require an O(n) search of the vector; perhaps a map or set would have been more appropriate.* The user interacts directly with raw pointers.* Lack of smart pointers makes it easy to forget to remember to increment/decrement the intrusive reference count (example: GetElement doesn't increment the counter).* Lack of refactoring in general.* No support for loading resources from a stream, which is restrictive.* No preemptive allocation (i.e. calling reserve()).* All handles are purely integer indexes to resource pointers, the user could just aswell keep hold of the pointers themselves.So what then instead?The manager here just seems to be both a resource loader and a pool of resources merged together.Had those two concepts been separated then the design would make much more sense, it could be more performant and simpler for the user to use and understand.[Edited by - dmatter on April 24, 2008 9:40:07 PM];Quote:Original post by swiftcoderThis design imposes a separate ResourceManager on every type of resource, which quickly multiplies, and is not ideal in a dynamic plugin-based environment. It also requires that every type of resource be derived from the resource class, which may well require a lot of jumping through hoops when integrating 3rd-party components.maybe the ResourceManager could also make use of the Singleton pattern, one and only one ResourceManager for all type of resource ?   ;  Hi, thanks for your views.It seems I sent a pre-release version of the article and code to the editor, my bad. I'll update the release version (merging with some changes based on the feedback, so unsigned ints won't become -1 and dangerous GetLists won't be there :) )I'll try to answer a few of the queries here though.1. Yes, a more advanced and performance enhancing implementation would have had a single resource manager for all types. We could enable the passing of destructor callbacks (just as custom CreateResource functions are callbacks called on object creation in the code), and use RTTI (either autogenerated or manually handled) to do that. However, the focus of the article is to get a resource manager up and running, while keeping in mind the major obstacles to tackle. It is more of a guide on how to get started, and by no means mean to be the ultimate manager. The implementation provided can be tweaked to support the above two, and be more useful - but someone who knows how to tweak it that way probably has his own resource manager :)2. @swiftcoder: name, path and handle are different for each resource. They are what differentiates one resource for another. They could be handled in the resource manager class, but then there would be the need for some sort of mapping. We decided to avoid this complexity.3. @swiftcoder: We were thinking of having an IRenderable interface, which the Mesh resource would implement. The renderer would delegate rendering to the mesh class' render method. I believe this is a pattern commonly used (and I have used it a lot myself).4. @dmatter: Searching by string is meant to be used only during insertion. A lookup by handle is O(1). The search by string is exposed only as a convenience, it could have easily been kept private/protected.5. @dmatter: Stream loading, smart pointers and preemptive allocation - yum yum. They gave way to simplicity, but they would be the obvious next steps. 6. @dmatter: Users could handle the pointers the pointers themselves, but the lookups would take iterating through a list/vector if no extra code was done to handle the pointers. The implementation does that in a consistent fashion and also handle reference counting. A lot of the other issues are already fixed in the final version and I'll hand it over to the editor soon.Thanks.   ;  I'm a newb (haven't worked on robust systems etc.), and although this isn't the first time I hear about using one resourcemanager to rule them all, I don't see why is that better? and how can it be done without deriving all my resources from a common abstract class?(some arguments I see:pro:* filenames are unique (they shouldn't get loaded in the wrong manager) -- this can be handled* no code bloating arising from template instantiation (this can be tackled -- my manager template class inherits privately from a manager core class, which works with pointers to the resource base class)con:* uses of resources are unique to their type -- what if I'm asking for a mesh from the one stop shop, and get a font, because I've had the wrong handle? (stupidity/typo/resources removed and loaded))p.s.: loved the article, especially the vector + stack solution.   ;  It's ironic that I should see this article, because for the last two days I've been converting an old VB6 implementation of this technique that I'd made some years ago to a more generalized .NET 2.0 version for use at work and home.  In my original version (and the current one, in fact), I used an array that was dynamically resized as needed, since VB6 didn't have anything like vectors; I referred to the handle as an "index pointer;" and I didn't implement a stack for dirty handles; instead, I had each node keep track of its adjacent nodes based on status (open or closed) and used this information to track the next open/closed node and traverse the list (backwards, incidentally), bypassing all nodes of the opposite type.  A lookup was (and is) O(n), where n is the number of closed nodes. My new implementation is a generic, so you can specify the data type.  (If you want to store any kind of object, simply declare a DualLinkedList(Of Object) and cast everything as type Object.  (Of course, you need to know how to change them back, which can be done by querying a common interface for a user-defined type.)  It also encapsulates more functionality -- all list resizing is handled internally, with the application programmer specifying a few parameters to optimize it -- and supports quite a few more methods, one of which is node consolidation:  relocating nodes to the front of the list and in sequential order, basically a form of smart defragmentation, but very slow and not to be used often.I like your use of a stack to store dirty handles, and although it isn't necessary for my use of index pointers -- the implementation of which I spent a day working out before I made my first resource manager; it uses a dual-linked list concept that is very complicated but requires minimal code and works very reliably -- I may adopt it for my own class and see how it goes.   ;  @trajectorymodifier:A "one manager to rule them all" is better from the viewpoint of someone integrating their code with third party tools, as swiftcoder pointed out. It has some performance benefits too. One may go about keeping run time type information (RTTI) and using that to make sure a Mesh is not a font. There are checks involved there - so there's a tradeoff.@tom:Interesting approach. I'd avoid casting to object when using it as boxing/unboxing is pretty expensive. I remember seeing a billboarding example in the DX SDK that had managed code running over 50% slower than the native one. Other samples boasted speeds of 95% of native code. The only reason that that particular example was so slow was that the billboarding example had to box and unbox. This was in .net 1.1, and in .net 2.0 we do have generics, but if we cast to and from object, the same penalties will hold.One of the main features of the article is the vector + stack. That is what enables access to resources by handles in O(1) time. Traversing will take O(n) or at best O(nlog(n)) (with a cool tree implementation and some headaches).I'd like to see your .net version when it's ready. Good luck :)   ;Quote:Original post by HeartattacK2. @swiftcoder: name, path and handle are different for each resource. They are what differentiates one resource for another. They could be handled in the resource manager class, but then there would be the need for some sort of mapping. We decided to avoid this complexity.4. @dmatter: Searching by string is meant to be used only during insertion. A lookup by handle is O(1). The search by string is exposed only as a convenience, it could have easily been kept private/protected.6. @dmatter: Users could handle the pointers the pointers themselves, but the lookups would take iterating through a list/vector if no extra code was done to handle the pointers. The implementation does that in a consistent fashion and also handle reference counting.In which case I would say that the ResourceManager has precisely zero reasons to exist. it doesn't perform any operations on the resources, and it doesn't provide any mapping, and it doesn't manage resource lifetimes (at least not as implemented). All it does is add an integer-to-pointer indirection (with associated performance cost), and no benefits whatsoever.Instead you might consider letting the ResourceManager store a map/hashmap of names to resources, and return the resource pointer directly. That way you provide a useful mapping, and eliminate the linear-search-by-name and the integer-to-pointer indirection. The mapping is still only needed at load time, and there is no performance penalty, and no need to track empty slots - ref-counting can still be done on add and remove.Quote:@swiftcoder: We were thinking of having an IRenderable interface, which the Mesh resource would implement. The renderer would delegate rendering to the mesh class' render method. I believe this is a pattern commonly used (and I have used it a lot myself).Being common does not make it a good idea - prefer composition to inheritance where ever possible. IrrLicht would be a good example of this gone wrong: ~10 different Mesh classes each rendering and animating itself in its own way, and thus no uniform interface to anything.Edit: In addition to dmatter's code critiques, there is no reason to hold a std::vector by pointer (it is basically a pointer to an array itself), especially when you hold the std::stack by value.   