Hi Thomas!

On 04/10/2008 06:56 AM Thomas Nunninger wrote:

> I really like the design (including your conclusions here in this 
> thread).

>>> [snip]

>>>> The restore() method does not work as complex as the store()
>>>> method does. Its algorithm is defined by the following pseudo
>>>> code: ::
>>>>
>>>>     $storage->lock();
>>>>     $item = $storage->restore(...);
>>>>
>>>>     if ( $item !== false )
>>>>     {
>>>>         // Notice access to this item in meta information
>>>>         $meta = $storage->restoreMetaInfo();
>>>>         update( $meta );
>>>>         $storage->storeMetaInfo( $meta );

>>> If $meta would be an object, or returned by reference, there would
>>> be no need for storeMetaInfo() right here.

>> I think we should keep it. The problem is that we need to restore the
>> meta info every time and store it again while the storage is locked.
>> If we obtain the storeMetaInfo() methods, there is no chance for the
>> storage to see when it needs to be stored again, except to
>> automatically store it inside unlock(). This is quite dirty and
>> untransparent, so I prefer to keep the calls.

> Besides that, I think, using an object everywhere you use the meta array 
> has several advantages. In my opinion you work with an array to often. 
> In the ezcCacheStackableStorage interface you define the methods 
> storeMetaInfo( array $metaInfo ). Also (I guess) restoreMetaInfo() 
> returns an array.

> In your pseudo code you restore and update the meta information several 
> times. So it could happen that this (big) array is copied several times 
> in a request. Wouldn't it be nicer, to work with an object so that only 
> the reference is provided as parameter or value in method calls?

In fact I already planned the meta data to be an object, as you can see
with the class ezcCacheStorageMetaInformation. However, it seems I
forget to update it everywhere.

> Also 
> it could be nice to put some of the meta information's logic into that 
> object, like $metaInformation->update( $itemIdentifier ). Implicitly 
> this would require an inherited class of ezcCacheStorageMetaInformation 
> for each replacement strategy. That way, the strategy does not need to 
> check a string in the meta information but an instance of.

The problem is, that you cannot require the storage to restore the
correct subclass when fetching the meta data again. For example the
ezcCacheStorageFileArray will store an array structure internally. If it
would need to store the class name in addition. While this works with
some quirks, I still prefer keeping 1 class since it means less overhead
in code.

> But even if you do not want to create an extended class for each 
> strategy, you still could use the ezcCacheStorageMetaInformation like 
> $meta->update( $itemIdentifier, $newValue ) or even 
> $meta->$itemIdentifier = $newTimestamp or $meta->$itemIdentifier++.

I don't see a sense here. $meta->data[$id]++ works as well. Why should
it offer access methods?

> Something about the restore() method:

>> $storage->lock();
>> $item = $storage->restore(...);
>>
>> if ( $item !== false )
>> {
>>     // Notice access to this item in meta information
>>     $meta = $storage->restoreMetaInfo();
>>     update( $meta );
>>     $storage->storeMetaInfo( $meta );
>> }
>>
>> $storage->unlock();
>> return $item;

> Is it really necessary to lock the storage for the whole thing or only 
> for updating the meta data? This would reduce the time, the storage is 
> locked a little bit...

It is. Imagine the following szenario, where LRU is used:

Item A is least recently used. Request 1 reads A. Request 2 stores B and
needs to replace A. After A is read by 1, 2 replaces A. After that 1
updates the meta info. This results in a completly unstable state.

> Another idea about locking: perhaps it's faster (at least in some cases 
> like many cached items on a file system based cache) to keep the meta 
> information in a database. restore() wouldn't need looking at all, just 
> one update SQL statement. And even for storing, I guess, this could be 
> faster or at least easier to implement an item based locking.

You'd need locking here, too, but on the database. However, if we need
to abstract the storage of meta data, this even adds more overhead as
the whole thing already does. Do we really want this?

> I do not ask you to implement this for now - just keep in mind when 
> implementing ;-)

It won't be too easy to add this later without breaking BC. Therefore we
should agree on a solution right now.

Thanks for your input, Thomas!
Regards,
Toby
-- 
Mit freundlichen Grüßen / Med vennlig hilsen / With kind regards

Tobias Schlitt (GPG: 0xC462BC14) eZ Components Developer

[EMAIL PROTECTED] | eZ Systems AS | ez.no
-- 
Components mailing list
Components@lists.ez.no
http://lists.ez.no/mailman/listinfo/components

Reply via email to