New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache Standard Interface #17

Closed
wants to merge 71 commits into
base: master
from

Conversation

Projects
None yet
@tedivm
Contributor

tedivm commented Mar 11, 2012

Proposed is a set of interfaces for a basic cache system, as well as extensions that can be used to build upon it. The goal of this PSR is to allow developers to create cache-aware libraries that can be integrated into existing frameworks and systems without the need for custom development.

Conversation has occurred on the list with regards to this, but additional discussion and PR's is highly encouraged.

tedivm and others added some commits Feb 25, 2012

Added initial outline and interfaces.
This is the initial outline of the caching proposal, including the
interfaces themselves (which are the most fleshed out part of this
being added to the repo at the moment).
Added main interfaces to their own files
This will allow people to actually use the interfaces.
Updated the format and added new sections
Changed some of the formatting around, added the "Multiple" section.
Expanded the PSR-Cache proposal.
Extensively expanded the proposal. Added the new interfaces (with
comments), wrote out the introduction and data sections, and cleaned up
a lot of formatting.
Updated the return value for Cache\Item->clear() function.
Clarified that the clear function should return true as long as the
item is not in the cache at the end of the call, regardless of whether
it was removed or never existed.
Expanded the PSR
Clarified acceptable keys, clarified that null should explicitly be
null, altered header names, added text for namespaces, added the
TaggableItem interface, and removed the driver interface.
Merged the Factory and IteratorFactory classes into the Pool class.
By merging the two classes into one we create more of an environment
class, which makes certain extensions significantly easier to architect
simply.
Cleanup and commenting.
Ran through and made sure all class names were correct and fixed some
misspellings. Cleaned up the interfaces a bit, and then brought their
new comments back into the main proposal.
Merge pull request #1 from maerlyn/Cache
php opening tags for syntax highlight
@nateabele

This comment has been minimized.

Show comment
Hide comment
@nateabele

nateabele Mar 15, 2012

+1 to pretty much everything about this.

nateabele commented Mar 15, 2012

+1 to pretty much everything about this.

Show outdated Hide outdated proposed/PSR-Cache.md
@@ -0,0 +1,311 @@
## Introduction
Caching is a common way to improve the performance of any project, making caching libraries one of the most common features of many frameworks and libraries. This has lead to a situation where many libraries roll their own caching libraries, with various levels of functionality. These differences are causing developers to have to learn multiple systems which may or may not provide the functionality they need. In addition, the developers of caching libraries themselves face a choice between only supporting a limited number of frameworks or creating a large number of adapter classes.

This comment has been minimized.

@ghost

ghost Mar 16, 2012

Could you please format this markdown file at around 80 characters?

@ghost

ghost Mar 16, 2012

Could you please format this markdown file at around 80 characters?

This comment has been minimized.

@tedivm

tedivm Mar 16, 2012

Contributor

Done.

@tedivm

tedivm Mar 16, 2012

Contributor

Done.

@norv

This comment has been minimized.

Show comment
Hide comment
@norv

norv May 27, 2012

I assume the "set" function referred here, is Item::set(). Please feel free to correct my understanding if that wouldn't be the case, I can see some problems if not though. Working on namespaces, I encounter the same question, and I'm specifying persistence as bound to Item::set() only.

I assume the "set" function referred here, is Item::set(). Please feel free to correct my understanding if that wouldn't be the case, I can see some problems if not though. Working on namespaces, I encounter the same question, and I'm specifying persistence as bound to Item::set() only.

This comment has been minimized.

Show comment
Hide comment
@tedivm

tedivm May 28, 2012

Owner

That is correct- the basic idea is that until an item is actually set with a value then nothing about that item needs to be written.

Owner

tedivm replied May 28, 2012

That is correct- the basic idea is that until an item is actually set with a value then nothing about that item needs to be written.

norv and others added some commits May 28, 2012

php tags in Extensions interfaces.
Signed-off-by: Norv <norv@simplemachines.org>
Updated the Stackable keys to have them start with a root slash.
This was brought up on the mailing list and seemed like a reasonable
idea. It also will make it easier for implementing libraries to use one
class for systems that use stackable and non-stackable components,
making it easier to segment the two pools (internal namespacing or
something) as there will *always* be a slash when Stackables are used.
Merge pull request #6 from tedivm/cache_stacks
Updated the Stackable keys to have them start with a root slash.
Clarification on Item::set() as the method binding to the underlying …
…system.

Added php tags.

Signed-off-by: Norv <norv@simplemachines.org>
Merge pull request #7 from norv/Cache
Clarification on Item::set()
Updated the names of the "get" functions to use "Item" instead of "Ca…
…che"

As discussed on the mailing list, there may be some confusion over the
"getCache" and "getCacheIterator" functions since there is no actual
cache object. By changing them to "getItem" and "getItemIterator" we
make it much more clear what is actually returned.
Merge pull request #8 from tedivm/cache_naming
Updated the names of the "get" functions to use "Item" instead of "Cache"
*
* @return bool
*/
function isMiss();

This comment has been minimized.

@mrclay

mrclay Nov 8, 2012

Almost all validators I've seen pass with a positive result, so I'd prefer to see this as isValid, isAvailable, etc.

@mrclay

mrclay Nov 8, 2012

Almost all validators I've seen pass with a positive result, so I'd prefer to see this as isValid, isAvailable, etc.

This comment has been minimized.

@mnapoli

mnapoli Nov 9, 2012

Member

+1 : if (! isMiss) is less readable than if (! isAvailable) (avoid double negatives)

@mnapoli

mnapoli Nov 9, 2012

Member

+1 : if (! isMiss) is less readable than if (! isAvailable) (avoid double negatives)

This comment has been minimized.

@mrclay

mrclay Nov 9, 2012

BTW I realize that you chose isMiss because most checks would be testing for a fail before regenerating the cached value, but I still think it looks funny in the interface.

@mrclay

mrclay Nov 9, 2012

BTW I realize that you chose isMiss because most checks would be testing for a fail before regenerating the cached value, but I still think it looks funny in the interface.

This comment has been minimized.

@dragoonis

dragoonis Dec 12, 2012

Member

I'm in agreement here. The isValid() or isAvailable() are much closer to what I'd expect from the proposal. @tedivm can you please update your PR to make this a true check rather than a false check?

@dragoonis

dragoonis Dec 12, 2012

Member

I'm in agreement here. The isValid() or isAvailable() are much closer to what I'd expect from the proposal. @tedivm can you please update your PR to make this a true check rather than a false check?

This comment has been minimized.

@tedivm

tedivm Dec 12, 2012

Contributor

Is here a preference? I'm thinking "isValid" is better than "isAvailable".

@tedivm

tedivm Dec 12, 2012

Contributor

Is here a preference? I'm thinking "isValid" is better than "isAvailable".

This comment has been minimized.

@Seldaek

Seldaek Dec 12, 2012

Contributor

how about exists()?

@Seldaek

Seldaek Dec 12, 2012

Contributor

how about exists()?

This comment has been minimized.

@tedivm

tedivm Dec 12, 2012

Contributor

I'm not a big fan of exists, as whether an item exists or is valid are really two separate questions. The "get" function may be able to return a value that exists, but is past it's ttl and therefore no longer valid, at which point the developers using the library needs to make a decision about what they'd like to do. In most cases they'll treat validity and existence as the same, but that won't always be the case.

@tedivm

tedivm Dec 12, 2012

Contributor

I'm not a big fan of exists, as whether an item exists or is valid are really two separate questions. The "get" function may be able to return a value that exists, but is past it's ttl and therefore no longer valid, at which point the developers using the library needs to make a decision about what they'd like to do. In most cases they'll treat validity and existence as the same, but that won't always be the case.

This comment has been minimized.

@Seldaek

Seldaek Dec 12, 2012

Contributor

Well if it's expired IMO it shouldn't exist anymore, it should be a
cache miss.

@Seldaek

Seldaek Dec 12, 2012

Contributor

Well if it's expired IMO it shouldn't exist anymore, it should be a
cache miss.

This comment has been minimized.

@mnapoli

mnapoli Dec 12, 2012

Member

IMO it shouldn't be possible to get a cache entry that expired.
Edit: I agree with @Seldaek

@mnapoli

mnapoli Dec 12, 2012

Member

IMO it shouldn't be possible to get a cache entry that expired.
Edit: I agree with @Seldaek

This comment has been minimized.

@tedivm

tedivm Dec 12, 2012

Contributor

90% of the time you're right, but I don't think we should exclude the 10% time when it's reasonable. There are many, many cases where it's more valuable to use a stale value (presumably while another process is regenerating the proper value), rather than enter a stampede (aka the dog pile affect) or pause to wait for the value to return.

Keep in mind this isn't saying get has to return anything- it doesn't. What this is saying is that the option is there for a library to expand it's functionality within the scope of this proposal. If a library wants to have a "standard" Pool and Item class, but also wants an extended Item class that handles validation differently (such as by doing any of the following- http://stash.tedivm.com/Invalidation.html ) then this standard should not get in their way.

@tedivm

tedivm Dec 12, 2012

Contributor

90% of the time you're right, but I don't think we should exclude the 10% time when it's reasonable. There are many, many cases where it's more valuable to use a stale value (presumably while another process is regenerating the proper value), rather than enter a stampede (aka the dog pile affect) or pause to wait for the value to return.

Keep in mind this isn't saying get has to return anything- it doesn't. What this is saying is that the option is there for a library to expand it's functionality within the scope of this proposal. If a library wants to have a "standard" Pool and Item class, but also wants an extended Item class that handles validation differently (such as by doing any of the following- http://stash.tedivm.com/Invalidation.html ) then this standard should not get in their way.

Removed Extensions from Proposal
The extensions were removed so the focus could be on the core standard.
Should this standard pass they'll be brought up for discussion and vote
on their own.
@nfx

This comment has been minimized.

Show comment
Hide comment

nfx commented Dec 6, 2012

tedivm and others added some commits Feb 26, 2013

Removed old PSR
Accidentally tossed an old PSR in here when merging with the main
fig-standards branch.
Changed the "isMiss" function to "isValid".
This was based off of Paul's suggestion in the mailing list, and tries
to make the function call more intuitive.
Changed the "flush" function to "empty"
This change was made to provide clarity, as some libraries use "flush"
to mean "save changes" which is pretty much the opposite of what's
happening here.
Merge pull request #10 from simensen/description
Added "Key Concepts" section to help explain naming conventions.
Merge pull request #12 from simensen/value-null
Value should be able to be set to null.
Merge pull request #13 from simensen/empty-to-clear
Use clear instead of empty as empty is a reserved word.
@philsturgeon

This comment has been minimized.

Show comment
Hide comment
@philsturgeon

philsturgeon Apr 25, 2013

Contributor

This proposal has been superseded by #96. Please continue discussion over on the mailing list topic.

Contributor

philsturgeon commented Apr 25, 2013

This proposal has been superseded by #96. Please continue discussion over on the mailing list topic.

dragoonis pushed a commit that referenced this pull request Jul 11, 2014

samdark pushed a commit that referenced this pull request Oct 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment