Some feedback:
1) You're doing two lookups into the map every time something is loaded. You can avoid this by using references to update in place. It also shortens the code. In this particular situation it doesn't matter too much since it's on the uncommon path, but it's a good technique in general.
// P is either the pre-existing entry, or a default constructed (i.e. NULL)
// Resource if it wasn't already in the map. Since P is a reference,
// we can update it in place.
Resource& P = resources[locator];
if (!P) {
Resource& loaded = LoadResource(locator);
if (loaded)
P = loaded
else
throw std::runtime_error("BORK!");
}
2) When iterating a container, it's generally best to cache the end iterator, as long as you're sure you won't be invalidating it. std::map isn't particularly bad, but in general it's hard to know how much work is involved in calling end(), and I've seen simple loops that ended up being ridiculously slow entirely because they called end() on every iteration. The idiom I typically use is below. It's a bit more complicated, but I've found it a good habit to get into to avoid surprises down the road.
// This assumes we're using a map where (...) doesn't invalidate
// arbitrary iterators. This is almost always the case with std::map
// (unless you do something really braindead), but most hash_maps
// offer weaker iterator guarantees, so be careful!
for (ResourceMap::iterator I = resources.begin(), E = resources.end(); I != E; ++I) {
...
}
3) This is a meta-comment, but std::map isn't really what you want in this situation, for several reasons.
First, because it's a tree-based structure, lookup time is logarithmic. So you're doing a logarithmic time operation each time you access a resource, even on the hot path. You really want to be using a hash map structure, which can take longer in the uncommon case when a resource is actually loaded, but is constant time for the common case. I think boost has hash maps, and it's not that hard to write your own.
Secondly, using the default comparison operators for maps of strings is generally a really bad idea, since comparing strings is linear in the length of the string. You really want to be using a hash map with a hash function specialized for strings so that you're not doing several linear-time string comparisons on each insertion.
Third, std::map is very copy-heavy. That is to say that any key or value used in the map will be duplicated many times. Thus, for something with comparatively expensive copy semantics like std::string, you'll be paying a non-trivial amount of time making needless copies of the string. You either want to use some sort of rope class that reduces the copy cost, or use a non-copying hash map.
The
LLVM source base has a very good
StringMap class that provides a very efficient string-to-T map that I usually pull in to projects where I need to do a significant amount of string mapping..
