Hi,

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? 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.

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++.


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...

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.

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


I'm looking forward to your implementation. Have a nice day

Thomas
-- 
Components mailing list
Components@lists.ez.no
http://lists.ez.no/mailman/listinfo/components

Reply via email to