[Scummvm-devel] ...to be or not to be?

[Max Horn]

Am 22.02.2009 um 00:48 schrieb sunmax at libero.it:

> Ciao Max,
>
> thanks for your prompt replies!
>
>
>>> I stay tuned for an optimized ontological solution ;-)
>>
>> I changed it on trunk (but not on the 0.13 branch, and that's
>> intentional) to only require 2.
>
> Great.
>
> One question: why do we need to check it more than once anyway?

Man, you are persistent, aren't you? :-). I finally managed to get it  
down to 1, though it might cause regression somewhere.

In brief, the answer to your question is: because our code defensively  
check(ed) in several places whether a file it was asked to open  
actually does exist. Based on the principle "if an error check is  
cheap, don't fret about doing it in several places if you can't be  
sure whether the caller already verified the validity".

My code changes essentially pushed down certain validations to a lower  
level, so that the higher level doesn't have to worry about it, as  
long as it validates the return result correctly. Drawback: In theory  
we now cannot print quite as accurate error messages anymore. But  
since we didn't print those before either, this isn't a real loss.

At the end of the day, though, we can't always avoid check for a  
file's existence multiple times.


>
>
>> As it is, our code relies on FSNode::exists() to be a fast
>> operation. Is this a problem for your port?
>
> It depends on the media: we have "stat" (it return bogus values
> but they are consistent, so I was able to remap their meaning)
> on mass: (USB) and pfs0: (HD). I did not benchmark them, but
> the feeling when browsing the files is that things are faster
> than with "open". On the other hand on CD and HOST (remote),
> "stat" is a dummy. So we have to resort to "open" and do some
> cabala to distinguish files from dirs, so not really fast.
>
> I thought we were caching the nodes in the game data folder,
> and just checking against the map thereafter. But maybe it
> is just in my head ;-)

We are caching the nodes! But this doesn't help with your problem  
here, because we only cache the nodes, not its attributes; doing that  
is up to the Node implementation, i.e., the backend author.


Bye,
Max