[Cache] set() vs. save() and future extension
119 views
Skip to first unread message
Larry Garfield
Jul 3, 2013, 3:01:39 AM7/3/13
to php...@googlegroups.com
Earlier, we had a bit of a back and forth regarding "where does the
save() method go?" I had forgotten when I started the poll earlier
that, in fact, the current proposal doesn't have a save() method
anywhere. :-) Item::set() is its own commit.
I'm not sure I like that. While it's certainly shorter to write this:
$pool->getItem('foo')->set('bar', 300);
Than either of these:
$pool->getItem('foo')->set('bar', 300)->save().
$pool->save($pool->getItem('foo')->set('bar', 300));
That assumes that only TTL is relevant, as TTL is set via the set()
method. This becomes more relevant when we consider extensions.
Consider for a moment the following naive tagging implementation:
$item = $pool->getItem('foo');
$item->set('bar', 300);
$item->addTag('narf');
In that case, the order matters because the tag comes after setting the
value and TTL. That's because set() is now acting as both "change a
value" and "commit to disk". Generally speaking, a method that does 2
things should be considered a code smell.
I've never seen a Domain Repository implementation where that was the
case. There's always been a save(), commit(), flush() or something like
that to actually persist changes. Since that's the model we're
following, we should fully embrace that.
Of course, that logic begs the question of why TTL is incorporated into
set() as well. At a fully abstracted level, we ought to be doing this:
$pool->getItem('foo')->setTtl(300)->set('bar')->addTag('narf')->save();
Which then implies that the following line, executed a moment later,
would reuse the same TTL and keep the same existing tag(s):
$pool->getItem('foo')->set('baz')->save();
Which is of course more consistent with a domain repository model, and
more extensible, but further afield from where we are now.
Of course, that logic applies just the same with save() on the pool.
The code samples then would look like:
$item = $pool->getItem('foo')->setTtl(300)->set('bar')->addTag('narf');
$pool->save($item);
// ...
$item = $pool->getItem('foo')->set('baz');
$pool->save($item);
But otherwise all of the same logic applies.
My tentative recommendation to resolve this issue would be as follows:
- Add Item::setTtl() and Item::getTtl() methods. getTtl() returns the
number of seconds that the cache item was supposed to live for, NOT how
many it has left. (getTtl() feels a little weird to me; feedback welcome.)
- Add Item::getExpiration(). It returns the DateTime at which the item
will expire.
- Leave the $ttl parameter on set() as a convenient shorthand, such that
the following two statements must be identical:
$item->setTtl(300)->set('bar');
$item->set('bar', 300);
- Add a save() method to persist a cache object. Syntactically from the
above it seems prettier to put it on Item, but I could be convinced
otherwise.
- Add a Pool::set($key, $value, $ttl). It MUST be exactly equivalent to
the following (and in most cases would probably be almost exactly this
code internally):
$item = $pool->getItem($key);
$item->setTtl($ttl)->set($value)->save();
return $item;
That way, people who don't want to be faced with the full details of a
domain caching model have a simple shorthand for the degenerate ttl-only
case (which is the common case). Anyone wanting something more complex
would have to use the more robust option.
That is, the "degenerate" case would sort of mimic a Weak Model, like so:
$item = $pool->getItem('foo');
if (!$item->isHit()) {
$item = $pool->set('foo', something_expensive(), 300);
}
return $item->get();
Which would be logically identical to:
$item = $pool->getItem('foo');
if (!$item->isHit()) {
$item
->set(something_expensive())
->setTtl(300)
->save();
}
return $item->get();
If you wanted to use tags, namespaces, or whatever else (assuming those
end up as separate methods), you'd have to use the second form. If you
only wanted TTL, you could use the first, condensed form if you were so
inclined.
That gives us a nice and simple syntax for the common case, but an
extensible and future-friendly option ready-to-go as soon as we or
specific implementations start adding other functionality.
The possible downside is that reusing TTL or tags or other
non-stored-value properties from a loaded item may seem weird, and I'm
not certain whether we should require them to be respecified or allow
them to be silently reused.
Thoughts?
--Larry Garfield
save() method go?" I had forgotten when I started the poll earlier
that, in fact, the current proposal doesn't have a save() method
anywhere. :-) Item::set() is its own commit.
I'm not sure I like that. While it's certainly shorter to write this:
$pool->getItem('foo')->set('bar', 300);
Than either of these:
$pool->getItem('foo')->set('bar', 300)->save().
$pool->save($pool->getItem('foo')->set('bar', 300));
That assumes that only TTL is relevant, as TTL is set via the set()
method. This becomes more relevant when we consider extensions.
Consider for a moment the following naive tagging implementation:
$item = $pool->getItem('foo');
$item->set('bar', 300);
$item->addTag('narf');
In that case, the order matters because the tag comes after setting the
value and TTL. That's because set() is now acting as both "change a
value" and "commit to disk". Generally speaking, a method that does 2
things should be considered a code smell.
I've never seen a Domain Repository implementation where that was the
case. There's always been a save(), commit(), flush() or something like
that to actually persist changes. Since that's the model we're
following, we should fully embrace that.
Of course, that logic begs the question of why TTL is incorporated into
set() as well. At a fully abstracted level, we ought to be doing this:
$pool->getItem('foo')->setTtl(300)->set('bar')->addTag('narf')->save();
Which then implies that the following line, executed a moment later,
would reuse the same TTL and keep the same existing tag(s):
$pool->getItem('foo')->set('baz')->save();
Which is of course more consistent with a domain repository model, and
more extensible, but further afield from where we are now.
Of course, that logic applies just the same with save() on the pool.
The code samples then would look like:
$item = $pool->getItem('foo')->setTtl(300)->set('bar')->addTag('narf');
$pool->save($item);
// ...
$item = $pool->getItem('foo')->set('baz');
$pool->save($item);
But otherwise all of the same logic applies.
My tentative recommendation to resolve this issue would be as follows:
- Add Item::setTtl() and Item::getTtl() methods. getTtl() returns the
number of seconds that the cache item was supposed to live for, NOT how
many it has left. (getTtl() feels a little weird to me; feedback welcome.)
- Add Item::getExpiration(). It returns the DateTime at which the item
will expire.
- Leave the $ttl parameter on set() as a convenient shorthand, such that
the following two statements must be identical:
$item->setTtl(300)->set('bar');
$item->set('bar', 300);
- Add a save() method to persist a cache object. Syntactically from the
above it seems prettier to put it on Item, but I could be convinced
otherwise.
- Add a Pool::set($key, $value, $ttl). It MUST be exactly equivalent to
the following (and in most cases would probably be almost exactly this
code internally):
$item = $pool->getItem($key);
$item->setTtl($ttl)->set($value)->save();
return $item;
That way, people who don't want to be faced with the full details of a
domain caching model have a simple shorthand for the degenerate ttl-only
case (which is the common case). Anyone wanting something more complex
would have to use the more robust option.
That is, the "degenerate" case would sort of mimic a Weak Model, like so:
$item = $pool->getItem('foo');
if (!$item->isHit()) {
$item = $pool->set('foo', something_expensive(), 300);
}
return $item->get();
Which would be logically identical to:
$item = $pool->getItem('foo');
if (!$item->isHit()) {
$item
->set(something_expensive())
->setTtl(300)
->save();
}
return $item->get();
If you wanted to use tags, namespaces, or whatever else (assuming those
end up as separate methods), you'd have to use the second form. If you
only wanted TTL, you could use the first, condensed form if you were so
inclined.
That gives us a nice and simple syntax for the common case, but an
extensible and future-friendly option ready-to-go as soon as we or
specific implementations start adding other functionality.
The possible downside is that reusing TTL or tags or other
non-stored-value properties from a loaded item may seem weird, and I'm
not certain whether we should require them to be respecified or allow
them to be silently reused.
Thoughts?
--Larry Garfield
Robert Hafner
Jul 3, 2013, 3:48:01 AM7/3/13
to php...@googlegroups.com
Couple of quick notes-
* getTTL should not exist. While some drivers may be forced to save the TTL because they don't have a built in mechanism for time based expiration, others don't have to do that. If the TTL isn't being saved there's no way to return it. At the same time this is a value explicitly set by the developer, so I'm not sure why they would need to return it.
* Tags are evil. I know this was just an example, and not really relevant, but I have trouble passing up an opportunity to mention that tags are vile things that have no place in caching.
* I'm honestly not sure how I feel about the "save" function. For some reason I don't like it, but I can't actually logic something up at the moment to justify why.
* Pool->save($item) presents all sorts of problem. What happens if we pull an Item out of one Pool and then save it into another Pool? Should it be deleted from the first Pool and saved in the second, or should it be saved in both? Throw an error? What is the benefit of having that type of functionality?
Robert
> 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/51D3CC53.2050607%40garfieldtech.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
Lukas Kahwe Smith
Jul 3, 2013, 3:51:13 AM7/3/13
to php...@googlegroups.com
On Jul 3, 2013, at 9:48 , Robert Hafner <ted...@tedivm.com> wrote:
> * Tags are evil. I know this was just an example, and not really relevant, but I have trouble passing up an opportunity to mention that tags are vile things that have no place in caching.
regards,
Lukas Kahwe Smith
sm...@pooteeweet.org
Robert Hafner
Jul 3, 2013, 4:47:51 AM7/3/13
to php...@googlegroups.com
Oh, I agree completely that this needs to be addressed. I've just banged my head against my desk trying to figure out a way to support tags with anything resembling decent performance and found it to be an absolute nightmare. If you look at most caching libraries out there now there is very little support for tagging, especially across all of their caching backends. I do think there are other ways to address the underlying problem that work better with caching- my bias isn't against group invalidation, it's just against most ways you'd have to implement tagging.
Although I don't want to go too far into it now (simply to keep us focused on getting this caching PSR passed), I really do want to immediately dive into this exact problem once the final version is passed. I already have a *draft* (don't judge it too harshly) written with ideas on how to address this problem ( https://github.com/tedivm/fig-standards/blob/Cache-Extensions/proposed/PSR-CacheExtensions.md ).
On the same subject, another topic I want to address (which I don't have anything written for at the moment) is providing a unified way to deal with customized cache miss handling. Basically, you do not want your cache to have a very expensive item simply disappear, causing all your requests to suddenly start running that expensive code at once and ruining your day. Finding ways to limit only one process from running that code- by setting a flag and using a default value, the stale value, or even forcing a miss on a single process *before* it expires- is going to be an important consideration in some future caching proposal.
Robert
Although I don't want to go too far into it now (simply to keep us focused on getting this caching PSR passed), I really do want to immediately dive into this exact problem once the final version is passed. I already have a *draft* (don't judge it too harshly) written with ideas on how to address this problem ( https://github.com/tedivm/fig-standards/blob/Cache-Extensions/proposed/PSR-CacheExtensions.md ).
On the same subject, another topic I want to address (which I don't have anything written for at the moment) is providing a unified way to deal with customized cache miss handling. Basically, you do not want your cache to have a very expensive item simply disappear, causing all your requests to suddenly start running that expensive code at once and ruining your day. Finding ways to limit only one process from running that code- by setting a flag and using a default value, the stale value, or even forcing a miss on a single process *before* it expires- is going to be an important consideration in some future caching proposal.
Robert
> --
> 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/DA39D6AF-2C7B-4D5A-8551-64272A870EDC%40pooteeweet.org.
> 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.
Beau Simensen
Jul 3, 2013, 12:44:50 PM7/3/13
to php...@googlegroups.com
On Wednesday, July 3, 2013 2:01:39 AM UTC-5, Larry Garfield wrote:
- Add Item::setTtl() and Item::getTtl() methods. getTtl() returns the
number of seconds that the cache item was supposed to live for, NOT how
many it has left. (getTtl() feels a little weird to me; feedback welcome.)
- Add Item::getExpiration(). It returns the DateTime at which the item
will expire.
I agree with Robert here that getting expiration or TTL is probably not something we should try to do. I don't think we will have visibility into this aspect of a cached item. If we want to break setTtl out, great, but I don't think getting it or an expiration back out again is something we should worry about.
- Leave the $ttl parameter on set() as a convenient shorthand, such that
the following two statements must be identical:
$item->setTtl(300)->set('bar');
$item->set('bar', 300);
I think I'd just drop the ttl as the second argument to set if we are going to add a setTtl method. I don't see a lot of point in having both.
- Add a save() method to persist a cache object. Syntactically from the
above it seems prettier to put it on Item, but I could be convinced
otherwise.
I think that having a method on the item is preferable to putting it on the pool. Whether it is save() or commit() or whatever I'm not too sure about yet.
- Add a Pool::set($key, $value, $ttl). It MUST be exactly equivalent to
the following (and in most cases would probably be almost exactly this
code internally):
$item = $pool->getItem($key);
$item->setTtl($ttl)->set($value)->save();
return $item;
...
I think this could be a slipper slope and I'm not sure how much I like it. On one hand, it could be pretty convenient. On the other, what if someone wants to just bump the TTL for an item? Or if they want to now be able to just delete an item w/o going through the whole domain repository model stuff?
TMTOWTDI can be useful, but I am not sure how far down this path we really want to go. One of the things I really liked about Robert's original model was that there was really only one place to specify the key (getItem or getItems) and only one place to set a value.
Thoughts?
One thing I think we should keep in mind is how multiples are handled. If we want to do some sort of batching with an object returned from getItems we'll want to make sure that the intention of something like $item->save() is meaningful across both concepts.
$pool->getItem('foo')->set('bar')->save(); // atomic?
$items = $pool->getItems(['foo']);
$items['foo']->set('bar')->save(); // maybe add to UoW?
$items->flush(); // flush UoW in batch?
This implies that there might be a separate Item implementation for both multiple and single item use cases. Which is fine, but it is something that might not be obvious when talking about multiple item handling.
Josh Hall-Bachner
Jul 3, 2013, 2:03:41 PM7/3/13
to php...@googlegroups.com
On Wednesday, July 3, 2013 12:01:39 AM UTC-7, Larry Garfield wrote:
I'm not a big fan of this. I don't feel like the benefit is worth either the complication this introduces or the confusion of roles it brings about.
- Add a Pool::set($key, $value, $ttl).
I do like moving setTtl() to a separate method and adding getExpiration() though; that will help this proposal lead by example for any future extensions.
On Wednesday, July 3, 2013 12:48:01 AM UTC-7, Robert Hafner wrote:
* I'm honestly not sure how I feel about the "save" function. For some reason I don't like it, but I can't actually logic something up at the moment to justify why.
In the most common use case it transforms a single call to set into a multiple-call process, which feels kind of awkward. I do think it's probably the best way to solve the extensibility problem though.
guilher...@gmail.com
Jul 4, 2013, 1:47:57 AM7/4/13
to php...@googlegroups.com
Hi,
Providing a CacheItem->save() violates SRP, where CacheItem is not only responsible to hold its state, but also to know how to persist itself.Also, objects that contains too many methods are bad in general thanks to our awesome memory manager in PHP. That's the reason why ActiveRecord implementations suffer on performance because of all 4 bytes of pointers to a method each PHP object instance have to store. When dealing with large volumes of Items, this will lead to a memory overhead we could/should avoid.
As of TTL, I already explained in another thread why it should be moved out of CacheItem.
--
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/1e731e9e-44d8-4e32-a9ca-fcd1e03cd6cc%40googlegroups.com.
Paul Dragoonis
Jul 4, 2013, 9:53:10 AM7/4/13
to php...@googlegroups.com
On Thu, Jul 4, 2013 at 6:47 AM, guilher...@gmail.com <guilher...@gmail.com> wrote:
Hi,Providing a CacheItem->save() violates SRP, where CacheItem is not only responsible to hold its state, but also to know how to persist itself.
First of all, by doing this you create an architectural bidirectional dependency. CacheItem cannot live without CacheDriver. If CacheItem is supposed to be a VO, then it should have no dependencies at all. CacheDriver already have to know about CacheItem, so why enforce a bidirectional dependency (which is the root of all evil *cough* unmantainable code) for the sole purpose of being handy?I'll try to expose my line of thinking another way. Do you guys think the Connection (Pool, CacheDriver, whatever) that is responsible to store/retrieve/remove from driver should delegate this operation to an Item where its purpose is to represent a single element of the cache connection? It seems the Connection is already responsible for this, so why delegate it to the entry representation?
Also, objects that contains too many methods are bad in general thanks to our awesome memory manager in PHP. That's the reason why ActiveRecord implementations suffer on performance because of all 4 bytes of pointers to a method each PHP object instance have to store. When dealing with large volumes of Items, this will lead to a memory overhead we could/should avoid.
As of TTL, I already explained in another thread why it should be moved out of CacheItem.
On the recent vote on Strong vs Weak, strong was the winner and it means the CacheItem is in charge of storage of itself too. I am not in favor personally of CacheItems handling their own persistence but the majority wins.
Cheers,--
On Wed, Jul 3, 2013 at 2:03 PM, Josh Hall-Bachner <charl...@gmail.com> wrote:On Wednesday, July 3, 2013 12:01:39 AM UTC-7, Larry Garfield wrote:I'm not a big fan of this. I don't feel like the benefit is worth either the complication this introduces or the confusion of roles it brings about.
- Add a Pool::set($key, $value, $ttl).
I do like moving setTtl() to a separate method and adding getExpiration() though; that will help this proposal lead by example for any future extensions.
On Wednesday, July 3, 2013 12:48:01 AM UTC-7, Robert Hafner wrote:
* I'm honestly not sure how I feel about the "save" function. For some reason I don't like it, but I can't actually logic something up at the moment to justify why.
In the most common use case it transforms a single call to set into a multiple-call process, which feels kind of awkward. I do think it's probably the best way to solve the extensibility problem though.
--To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/1e731e9e-44d8-4e32-a9ca-fcd1e03cd6cc%40googlegroups.com.
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.
Guilherme Blanco
MSN: guilher...@hotmail.com
GTalk: guilhermeblanco
Toronto - ON/Canada--
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/CAFp73XsmTR0drBTMjuhf6xiKU99MjE-A1XLpV55ZnOAC%3Dt-nGA%40mail.gmail.com.
Paul Dragoonis
Jul 4, 2013, 9:54:42 AM7/4/13
to php...@googlegroups.com
On Wed, Jul 3, 2013 at 8:01 AM, Larry Garfield <la...@garfieldtech.com> wrote:
Earlier, we had a bit of a back and forth regarding "where does the save() method go?" I had forgotten when I started the poll earlier that, in fact, the current proposal doesn't have a save() method anywhere. :-) Item::set() is its own commit.
I'm not sure I like that. While it's certainly shorter to write this:
$pool->getItem('foo')->set('bar', 300);
Than either of these:
$pool->getItem('foo')->set('bar', 300)->save().
$pool->save($pool->getItem('foo')->set('bar', 300));
Isn't Pool->save() aka Pool->set() what was voted against?
--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+unsubscribe@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/51D3CC53.2050607%40garfieldtech.com.
guilher...@gmail.com
Jul 4, 2013, 10:16:48 AM7/4/13
to php...@googlegroups.com
Paul,
Strong vs. Weak had nothing to do with CacheItem->save() or not.To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAKxcST8VbQcuMyijCYEm6Bbo5WZorNuSm7OUGWfPmw9tvBmtQw%40mail.gmail.com.
Paul Dragoonis
Jul 4, 2013, 10:18:26 AM7/4/13
to php...@googlegroups.com
On Thu, Jul 4, 2013 at 3:16 PM, guilher...@gmail.com <guilher...@gmail.com> wrote:
It was towards having a Pool->set($key, $value, $tll) or Pool->set(CacheItem $item) and derivate methods.Paul,Strong vs. Weak had nothing to do with CacheItem->save() or not.
I did ask multiple times for someone to explain the diff to me in code, thanks for that.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAFp73Xtsx-2PdzFtxd13pfSS36aCOV5LNAvq_gE_DEBe%2BAYiuQ%40mail.gmail.com.
Larry Garfield
Jul 4, 2013, 2:04:33 PM7/4/13
to php...@googlegroups.com
On 07/04/2013 09:18 AM, Paul Dragoonis
wrote:
On Thu, Jul 4, 2013 at 3:16 PM, guilher...@gmail.com <guilher...@gmail.com> wrote:
It was towards having a Pool->set($key, $value, $tll) or Pool->set(CacheItem $item) and derivate methods.Paul,Strong vs. Weak had nothing to do with CacheItem->save() or not.
I did ask multiple times for someone to explain the diff to me in code, thanks for that.
Yeah, it wasn't as much a poll on "where does save() go" as much as
"key/value model vs. Domain Object model." I've seen domain objects
that have their own save() method (Drupal's for instance), which in
practice is often just a convenience wrapper:
public function save() {
return $this->repository->save($this);
}
I can live with save() in either location; that's not the point of this thread as much as the larger question about "set() and save()" vs. setAndSave(), which is much more fundamental. (And a method whose proper description contains "And" is a code smell, IMO.)
--Larry Garfield
public function save() {
return $this->repository->save($this);
}
I can live with save() in either location; that's not the point of this thread as much as the larger question about "set() and save()" vs. setAndSave(), which is much more fundamental. (And a method whose proper description contains "And" is a code smell, IMO.)
--Larry Garfield
Handrus Nogueira
Jul 5, 2013, 2:51:57 PM7/5/13
to php...@googlegroups.com
I couldn't agree more with Guilherme Blanco here.
We should avoid over-complicating our architecture.
--
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/51D5B931.4000600%40garfieldtech.com.
Handrus Stephan Nogueira
Kinn Julião
Jul 5, 2013, 7:43:03 PM7/5/13
to php...@googlegroups.com
For me, Pool->set($key, $value, $tll) makes more sense againts Pool->set(CacheItem $item)
The cacheItem is supposed to be anything... the cache layer should receive any value and store it...
The cacheItem is supposed to be anything... the cache layer should receive any value and store it...
so Cache->set("Bisna", "{my:'json'}", 3600) or Cache->set("Overenginner", false, 1) is much better than
Cache->set(CacheItem) and CacheItem has anyother attributes with anyother stuff...
Cache->set(CacheItem) and CacheItem has anyother attributes with anyother stuff...
IMHO, the cache should be simple as it name says... we don't want Interfaces to create Onion Softwares(lots of layers), we just want interoperability between modules to make things easier!
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAA88B97WR0VwKjVfRJn4Syq04HKKOHzNM5Wx-tm1e8F2Kw2E5w%40mail.gmail.com.
---------------------------------------------------------
Kinn Coelho Julião
---------------------------------------------------------
guilher...@gmail.com
Jul 6, 2013, 12:02:23 AM7/6/13
to php...@googlegroups.com
Hi Kinn,
So should I imply also that you prefer get($key) === $value too?To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAF%2Bu_BzW2mk3ua664iVTVo8QaHtwr5ktDSoPZJWiQqpHDbKO%3Dg%40mail.gmail.com.
--
Amy Stephen
Jul 6, 2013, 10:51:47 AM7/6/13
to php...@googlegroups.com
I use an exception for a miss.
I share Kinn's concerns (and love for a little fun sarcasm in code examples), however, I have so much respect for the knowledge and skill of those advocating the Cachitem approach that I'm interested in learning why and how it's more flexible and extensible, and why that would be useful.
In the end, the lines of code difference is negligible and flexible approaches tend to have applicability elsewhere. My thinking is a penny is going to drop for me when we see it all come together.
I share Kinn's concerns (and love for a little fun sarcasm in code examples), however, I have so much respect for the knowledge and skill of those advocating the Cachitem approach that I'm interested in learning why and how it's more flexible and extensible, and why that would be useful.
In the end, the lines of code difference is negligible and flexible approaches tend to have applicability elsewhere. My thinking is a penny is going to drop for me when we see it all come together.
Kinn Julião
Jul 6, 2013, 12:13:52 PM7/6/13
to php...@googlegroups.com
Once I'm using cache, I don't want to receive a "miss".
If I receive a "miss", for me, it is an exception.
If I receive a "miss", for me, it is an exception.
How can you prevent the miss with the cacheItem?
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAFp73XsnX6J5B1QFPAJASfrN29fHGPUt%3D3aQXiDL%3DNiqNczzZA%40mail.gmail.com.
Kinn Julião
Jul 6, 2013, 12:15:42 PM7/6/13
to php...@googlegroups.com
Same point as me Amy, exception.
I always use "sarcasm" to talk with Blanco, we are drunk Brazilians!
Anyway, if the point was the cache miss, I want to see the approach how the cacheItem can handle the "miss"
Thanks folks!
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/39f4e2dc-18da-4692-bd79-3ca0b9f87c77%40googlegroups.com.
FGM at GMail
Jul 6, 2013, 1:03:21 PM7/6/13
to php...@googlegroups.com
IMHO, when using a cache, a miss is an expected result, not something unusual, so there is no reason why it would have to throw an exception: by nature, caches are not persistent storage so any once-valid key can suddenly be pointing to missing data, even if the caching has been set as perpetual, notably because of evictions. This is a normal situation.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAF%2Bu_BxcdkakuVHcOfdh78QAx6hA1%2B8KmduSEkXo4Q9__3%2BJ9A%40mail.gmail.com.
Kinn Julião
Jul 6, 2013, 1:20:32 PM7/6/13
to php...@googlegroups.com
So, ok. How the CacheItem works with that?
If it's a plan value, as blanco said it can be null... If it's a CacheItem, the value will be null... So?
I will insist in exceptions...
Why not? OutOfBounds still there!
--
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/CAF%2Bu_BxcdkakuVHcOfdh78QAx6hA1%2B8KmduSEkXo4Q9__3%2BJ9A%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.
--
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/CANqRZ1OtAH4zbC3NnfumS8qrhjhVHLurPV-tw1j0FkBAWTwFmg%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.
Larry Garfield
Jul 6, 2013, 2:34:09 PM7/6/13
to php...@googlegroups.com
Renaming thread since this is no longer
about set vs. save...
That's exactly what the CacheItem is for. It is NOT simply a dumb wrapper. It carries useful metadata.
$item = $pool->get('foo');
if ($item->isHit()) {
// We know that a cached value was found.
// It may be NULL, but we know that it's a meaningful NULL.
}
else {
// We know that no cached item was found. That is to be expected
// when there are TTLs involved, as we want misses to happen so that
// we know to regenerate the value. As FGM said, a cache miss is not
// an abnormal or exceptional event so an Exception is inappropriate.
// Remember, throwing an exception is actually quite expensive since
// PHP has to generate a full stack trace to do so. It shouldn't be used
// for mundane flow of control.
}
We already agreed to move forward with the "Strong"/"Smart"/"Rich"/Whathaveyou Item model, because it gives us far more control and far more flexibility/extensibility. Now we're just trying to work out the details of that, and I think we're not far off. (Famous last words, maybe...) Let's stay focused on that.
--Larry Garfield
That's exactly what the CacheItem is for. It is NOT simply a dumb wrapper. It carries useful metadata.
$item = $pool->get('foo');
if ($item->isHit()) {
// We know that a cached value was found.
// It may be NULL, but we know that it's a meaningful NULL.
}
else {
// We know that no cached item was found. That is to be expected
// when there are TTLs involved, as we want misses to happen so that
// we know to regenerate the value. As FGM said, a cache miss is not
// an abnormal or exceptional event so an Exception is inappropriate.
// Remember, throwing an exception is actually quite expensive since
// PHP has to generate a full stack trace to do so. It shouldn't be used
// for mundane flow of control.
}
We already agreed to move forward with the "Strong"/"Smart"/"Rich"/Whathaveyou Item model, because it gives us far more control and far more flexibility/extensibility. Now we're just trying to work out the details of that, and I think we're not far off. (Famous last words, maybe...) Let's stay focused on that.
--Larry Garfield
Amy Stephen
Jul 6, 2013, 2:44:31 PM7/6/13
to php...@googlegroups.com
On Saturday, July 6, 2013 1:34:09 PM UTC-5, Larry Garfield wrote:
// We know that no cached item was found. That is to be expected
// when there are TTLs involved, as we want misses to happen so that
// we know to regenerate the value. As FGM said, a cache miss is not
// an abnormal or exceptional event so an Exception is inappropriate.
// Remember, throwing an exception is actually quite expensive since
// PHP has to generate a full stack trace to do so. It shouldn't be used
// for mundane flow of control.
Thanks Larry. Lead on!
}
See what I mean, Kinn? I think it's heading the right way. ;-)
// and they say geeks can't communicate effectively.
// the code examples and comments in this thread, alone, prove differently.
Kinn Julião
Jul 6, 2013, 5:09:31 PM7/6/13
to php...@googlegroups.com
So, why not a simple DTO, and a simple value to store?
Following the DTO example, now you can convince me in this way
Following the DTO example, now you can convince me in this way
$pool->get('foo', 'bar');
$item = $pool->get('foo');
if ( ! $item->isHit()) {
// We know that a cached value was not found.
// We know that a cached value was not found.
//Do something
}
}
return $item->getValue();
Following the original idea ,the cache item has some attributes and methods as isHit()
The object that I want to store is an instance of the following object:
class User
{
public $email;
public $id;
public $name;
}
If I want to store User, I don't want to create a CacheItem with isHit and anyother attributes... I just want to store User
If I want to store User, I don't want to create a CacheItem with isHit and anyother attributes... I just want to store User
$cache->set('user1', $user);
I don't need to create a CacheItem, with $user... the Cache layer should retrieve this to me as a DTO.
It should be simple!! $cache->set("key","value"); $cache->get("key");//return User $user
Anyway, as you said, it was already decided, so I'll not waste your time...
But it would be good to know WHY the cached value was not found (miss)... and the exception can provide this...
But it would be good to know WHY the cached value was not found (miss)... and the exception can provide this...
--
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/fe8453ae-d656-476b-b05d-47302404c8d4%40googlegroups.com.
Larry Garfield
Jul 6, 2013, 5:43:29 PM7/6/13
to php...@googlegroups.com
Suppose that generating a User object
is expensive (which is why we're caching it). And determining
that there is no user object to generate is also expensive. Say,
you're getting users from an LDAP server far away.
User 123 doesn't exist in the LDAP server. So it's NULL.
Someone asks for User 123. The cache then does this:
$user = $pool->get('user_123');
if (!$user) {
$user = crazy_expensive_ldap_call();
$pool->set('user_123', $user);
}
return $user;
So now you get back a NULL from the pool. You then don't know if that's because the cache doesn't know about it or because it knows there is no such user. So you call that crazy expensive operation again, get a NULL, re-cache a NULL, and return it. And then you do the same thing over again on the next call, thrashing the cache.
(A user is probably not the best example here, but plenty of data objects would follow this pattern.)
If instead you get back a CacheItem object, you can differentiate between "I found a NULL" and "I found nothing" and skip that stampeed.
Multiple existing cache implementations have solved this problem in basically the same way. (Stash and Drupal are the ones I know of off hand.)
Using an exception to indicate a cache miss is, as noted before, both semantically wrong (cache misses are normal) and slow (because PHP). If you really care why there was a miss, there's no reason a given implementation couldn't add such methods to its CacheItem (since the CacheItem and Pool have to come as a matched set):
$item = $pool->get('user_123');
if (!$item->isHit()) {
if ($item->isExpired()) {
// It was old, so we can safely regenerate it.
}
if ($item->invalidData()) {
// We couldn't retrieve it because the value was not serializable in the first place, dummy!
}
if ($item->isDay('Tuesday')) {
// We don't allow the cache to work on Tuesdays. Because... Tuesday.
}
}
(One could probably argue that there are additional metadata methods we could add to CacheItem, besides isDay() of course. That would be a separate thread.)
Really, caching is more involved and complex than just a k/v index. It doesn't seem like it at first, but one of the things that's been born out in this discussion over the last year is that doing it right takes more than a trivial k/v.
--Larry Garfield
User 123 doesn't exist in the LDAP server. So it's NULL.
Someone asks for User 123. The cache then does this:
$user = $pool->get('user_123');
if (!$user) {
$user = crazy_expensive_ldap_call();
$pool->set('user_123', $user);
}
return $user;
So now you get back a NULL from the pool. You then don't know if that's because the cache doesn't know about it or because it knows there is no such user. So you call that crazy expensive operation again, get a NULL, re-cache a NULL, and return it. And then you do the same thing over again on the next call, thrashing the cache.
(A user is probably not the best example here, but plenty of data objects would follow this pattern.)
If instead you get back a CacheItem object, you can differentiate between "I found a NULL" and "I found nothing" and skip that stampeed.
Multiple existing cache implementations have solved this problem in basically the same way. (Stash and Drupal are the ones I know of off hand.)
Using an exception to indicate a cache miss is, as noted before, both semantically wrong (cache misses are normal) and slow (because PHP). If you really care why there was a miss, there's no reason a given implementation couldn't add such methods to its CacheItem (since the CacheItem and Pool have to come as a matched set):
$item = $pool->get('user_123');
if (!$item->isHit()) {
if ($item->isExpired()) {
// It was old, so we can safely regenerate it.
}
if ($item->invalidData()) {
// We couldn't retrieve it because the value was not serializable in the first place, dummy!
}
if ($item->isDay('Tuesday')) {
// We don't allow the cache to work on Tuesdays. Because... Tuesday.
}
}
(One could probably argue that there are additional metadata methods we could add to CacheItem, besides isDay() of course. That would be a separate thread.)
Really, caching is more involved and complex than just a k/v index. It doesn't seem like it at first, but one of the things that's been born out in this discussion over the last year is that doing it right takes more than a trivial k/v.
--Larry Garfield
Amy Stephen
Jul 6, 2013, 5:47:32 PM7/6/13
to php...@googlegroups.com
Larry and FGM are right. There are many times that cache will not be found. For example, let's say a visitor requests an old article. I really had not considered Larry's point that generating an exception is costly. To me, it was a way to avoid a false or null value -- which as stated above could be a valid cached value.
Their approach takes care of both -- in using an object for item, an isHit property can be set to true or false and then a values field can be used for the cache value. That way, if it's false or null, the values can still be valid and a clear indication of whether or not a cache value was retrieved can be provided. All without the cost of the exception.
Makes sense.
(I see Larry just posted, too, explaining more clearly, but I'll still share my response since I now understand the point.)
Thanks Kinn.
Their approach takes care of both -- in using an object for item, an isHit property can be set to true or false and then a values field can be used for the cache value. That way, if it's false or null, the values can still be valid and a clear indication of whether or not a cache value was retrieved can be provided. All without the cost of the exception.
Makes sense.
(I see Larry just posted, too, explaining more clearly, but I'll still share my response since I now understand the point.)
Thanks Kinn.
Kinn Julião
Jul 6, 2013, 5:52:47 PM7/6/13
to php...@googlegroups.com
"add such methods to its CacheItem" make sense.
I'll enjoy this.
--
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/1fc36f91-719d-4685-91cf-12b65e4faa93%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Evert Pot
Jul 6, 2013, 7:10:51 PM7/6/13
to php...@googlegroups.com, php...@googlegroups.com
I would hate to prolong this conversation further, because i know it was covered many times before..
But in your specific example one could store 'false' for a non existent user.
--
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/51D88F81.8080405%40garfieldtech.com.
FGM at GMail
Jul 7, 2013, 3:32:01 AM7/7/13
to php...@googlegroups.com
There is also another issue, regarding multiple operations. Assuming you requested multiple keys and one of them only resulted in a miss, you still have to throw an exception and filter the hit results out of the exception data, or throw away the whole set, which would be a waste.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CD73E675-660F-41E0-8F28-80E3681829D6%40gmail.com.
Reply all
Reply to author
Forward
0 new messages