-
Notifications
You must be signed in to change notification settings - Fork 73
DOC Document query caching #771
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
DOC Document query caching #771
Conversation
michalkleiner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads well 👍
| - [`delete()`](api:SilverStripe\ORM\DataObject::delete()) | ||
| - [`destroy()`](api:SilverStripe\ORM\DataObject::destroy()) | ||
| - [`flushCache()`](api:SilverStripe\ORM\DataObject::flushCache()) | ||
| - One of the following is called on a `DataList` that manages that class or one of its subclasses: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text obviously works ok without the suggestion, not sure if it was intentionally left out or an oversight, so I thought I'd flag it for consideration.
| - One of the following is called on a `DataList` that manages that class or one of its subclasses: | |
| - One of the following methods is called on a `DataList` that manages that class or one of its subclasses: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
en/08_Changelogs/6.1.0.md
Outdated
|
|
||
| There are some caveats to this, along with some other options you might want to set, so check out the [indexes documentation](/developer_guides/model/indexes/) for more details. | ||
|
|
||
| ### Database query caching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this should be in the performance section then it should have a #### level heading to match the others
Though I don't this is a performance improvement by itself so should be in a different section? (unless I've misunderstood the PRs?) Seems like the PRs are adding a cleaner API that developers can use, though we haven't actually added any additional caching so "out of the box" application performance will be identical? The deprecated API like get_one() has been converted to use new API, and the issue in the original description SearchFilter::getCaseSensitiveByCollation() already had caching on a private static property?
All the other things in the performance section will make the CMS run faster after composer update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this should be in the performance section then it should have a
####level heading to match the others
Good catch. Updated heading to ####
Though I don't this is a performance improvement by itself so should be in a different section?
I guess that depends on how we're categorising "performance improvement" or "performance enhancement".... I've taken it to mean any changes we're making that are performance related, which a new caching functionality is.
58459ca to
0724034
Compare
Note that this is a performance enhancement so it belongs in the "performance improvements" section of the changelog - but it is also the sort of thing we want people to be aware of, so it should be in the table of contents. I've raised #770 to resolve this conundrum.
Issue
DataListsilverstripe-framework#11767