[Scummvm-devel] PS2 #9 : performance : node optimization : important

[Max Horn]

Am 02.03.2009 um 17:33 schrieb sunmax at libero.it:

>
>>> I don't think mixing general and backend-specific flags in a single
>>> field is a good idea, because it exposes backend-specifics to
>>> engines, and also forces a specific way of caching these values on
>>> the backends.
>
>>> I agree with Willem here, I guess you can just do it inside your own
>>> FSNode implementation. The isDir etc. API is IMHO way nicer than a
>>> getFlags method anyway.
>
> I actually agree with all of you guys, and the issue was flagged not
> cause I wanted to improve the API (which is surely cleaner this way)
> but because I am concerned about performance.
>
> Some (of the not so many) PS2 ScummVM users are reporting that they
> are switching to pre-FSNode ScummVM (things like 0.9.0) cause it's
> faster - not that they know the internal details, but is starts
> faster and games feel faster [on the other hand they did not try
> 0.13.x yet - so I should personally run a 0.9.0 vs 0.13.x match
> before even speaking...]
>
> So I was just wondering if a dirtier but more compact API with
> less methods and members could be faster when creating and
> deleting FSNodes than a cleaner one, even if I am sure we will
> never match a pure fopen C call.


If this is your concern, then your suggestion will not address it: If  
the creation&deletion is causing a slow down, then thigns won't get  
faster if you replace exists() by "getFlags() & NODE_EXISTS", obviously.

Rather, we should investigate what causes all those nodes to be  
allocated, if there is duplication that can be avoided, etc.:

* We scan various directories recursively: gamepath, themepath,  
extrapath, possibly the current directory. If there are many files in  
there, that could hurt performance. We should try to reduce the  
scanning to a minimum (that was one of the ideas behind SearchMan,  
actually: It performs a scan once, and caches the result)

* We could revise the FilesystemFactory API to return  
Common::SharedPtr<AbstractFSNode> instead of AbstractFSNode* in all  
methods. That would enable the following:

** Using class ObjectPool in common/memorypool.h to make a pool of  
FSNode objects, thus avoiding many new/delete calls. Requires using a  
custom SharedPtrDeletionInternal subclass, in order to make sure the  
object memory is returned to the pool

** Independently of the previous point, if we return  
SharedPtr<AbstractFSNode>, you could keep an internal cache of your  
PS2FSNode subclasses in your factory. If the total number of files is  
small, then you could keep one FSNode for each in memory.


Bye,
Max