[Cache] More updates

[Larry Garfield]

Right then. Headaches are contained for the time being, so...

* I have rearranged the code considerably. Specifically, the interfaces
now live, for now, ONLY in:

https://github.com/Crell/Cache

The utility classes now live ONLY in:

https://github.com/Crell/Cache-Util

(Both of those will move to the FIG organization once the spec is approved.)

The PR itself now contains ONLY the non-interface parts:

https://github.com/php-fig/fig-standards/pull/149

Maintaining the interfaces in 2 places was just too much of a hassle, so
I've split the spec for now. I will merge the repo code back into the
spec proper when it's ready to send to Paddy for final review; not
before. Hopefully this will also keep people from commenting on the PR
instead of here. :-) (I know, hope springs eternal.)

* I've renamed the interfaces as discussed previously. However, if
"pool" is so confusing that we have to go with "cache-item-pool", that
to me says pool is the wrong noun to begin with. In Drupal we've used
"bin" for years without issue. I recommend we switch to that name as
CacheBin is quite self-explanatory. Thoughts?

* The Collection support that was included previously was based on prior
discussions on the list from last fall. Apparently there's enough
people that dislike that approach that I've pulled it back out. Please
suggest an alternative. :-)

* There have been a few other tweaks based on feedback here and on the
PR. See the commit log for details. Of particular note, I kept the
split of set() and save() that was introduced with the collection
support. For one, that part did seem to be fairly popular. For
another, I do want to get multi-support in (as do most people per
previous poll), and having that split is mandatory for that to work. No
sense pulling it out just to put it back in again.

* While doing the memory implementations for the collection support, I
began to question the use of save() on the item. For multi-set to work,
we by definition have to dissect the item object and run the save
routine *in the pool*. Just iterating over the items and calling save()
doesn't buy us any performance benefit on multi-set-capable backends.
So if the pool is going to have to be able to dissect an item and save
it itself, then saving a single item is... exactly the same. ("One is a
special case of many.") That makes Item::save() redundant. I will write
more on this later, but for now, any new proposal for multi-handling
should include at least a memory implementation to see if it actually
makes sense in practice.

--Larry Garfield

[Josh Stuart]

Hi there,

I'm pretty new to all of this so apologies if I ask / comment on something that has already been discussed.

I spent the day yesterday porting my caching module over to your proposed spec: https://github.com/crimsonronin/juggernaut/tree/Enhancement/PsrCache It's just a work in progress but I'd appreciate any feedback, in particular around the `ChainedCachePool`.

I haven't come up against any real limitations of the spec yet, however, as I said, it's only been a day. Maybe you guys will see code in my implementation that doesn't make sense.

I'll attempt to define a multi set/get example over the coming weeks.

* re; nomenclature: I'd be hesitant to use `bin`, however that is for cultural reasons (in Australia a "bin" is a trash can, hence it insinuates something you don't want). I haven't gone through every thread, but what about `CacheManager`? (I'm sure it's been suggested). I don't mind the use of `Pool` though.

Cheers

[Phil Sturgeon]

Looks great Larry. It seems like most peoples concerns have been taken care of? 

[Larry Garfield]

Not entirely, because we still don't have multi-support figured out.  The previous poll implied a strong preference for getting that sorted out for the base implementation so I want to get that taken care of.  I tried implementing what the group discussed last fall and people didn't seem to like it.  So, someone suggest something else. :-)

Also, as noted I am growing more skeptical of the save() on Item.  When I tried to implement multi-support it felt very out of place.

--Larry Garfield

[Josh Stuart]

I think you are spot on Larry regarding the save() method, particularly when trying to implement a multi set /get approach.

As discussed previously (https://groups.google.com/d/msg/php-fig/_HMACz6NzrU/Z-mhzo7AfiIJ), we are treating CacheItem as a hybrid controller / model and I couldn't agree more with Guilherme. I went back through the posts but couldn't find a definitive reason why this was implemented?

An example where this becomes particularly messy is when trying to prime cache. eg.

$pool = new CachItemPool;

$primeData = getPrimeData();

foreach($primeData as $data) {

    $item = new CacheItem($pool);

    $item->set($data, $ttl);

    $item->save();

}

Larry, I think I read somewhere that you said you shouldn't directly instantiate a CacheItem, which makes the aforementioned example incorrect. 

I thought a more appropriate way would be to:

$pool = new CachItemPool;

$primeData = getPrimeData();

foreach($primeData as $data) {

    $item = new CacheItem;

    $item->set($data, $ttl);

    $pool->add($item);

}

$pool->flush();

Obviously this is the doctrine approach and the CachItems end up being just a direct implementation of the interface. Also, another con (or pro, depending how you look at it) is that a pool implemented in this manner ends up being a Memory cache of it's own since it stores references to all the cache items under management.

I just can't see a way to cleanly implement multi set/get using the current bi-directional method with CacheItems responsible for persisting themselves.

[Josh Stuart]

Sorry I should of mentioned those examples were using the current proposed spec (ie. disregarding a setMulti implementation), but it does highlight the real need for multi support.

Also apologies for raising the same concerns as expressed in https://docs.google.com/a/zoopcommerce.com/spreadsheet/ccc?key=0AsMrMKNHL1uGdDdVd2llN1kxczZQejZaa3JHcXA3b0E#gid=0 (I only just found it).

[Larry Garfield]

The primary reason, in my mind, to forbid instantiating CacheItem is extensibility.  Some implementations will extend the base interfaces to support, say, cache tagging and others will not.  That means CacheItem will need to be extended with extra methods.  If you're creating it yourself then it becomes much harder to handle swappable implementations, since you're hard coding the Item concrete class but not the pool concrete class.  They need to come as a matched set.

I know Robert is a fan of CacheItem being responsible for actually reading/writing from the data store (DB, memcache, etc.), as it allows lazy-loading.  However, with CacheItem always created by the pool that becomes an implementation detail and not a requirement of or forbidden by the PSR. 

Any suggestions on multi-set implementations that would work? :-)

--Larry Garfield

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/fd51e003-cf11-4381-a954-596895044984%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.