Skip to content
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

Improve Cache Hits - Feature Request #2786

Open
TimDix opened this issue Jul 23, 2015 · 5 comments
Open

Improve Cache Hits - Feature Request #2786

TimDix opened this issue Jul 23, 2015 · 5 comments
Labels
Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality of Life:3 Enhancements that will have a significant improvement in quality of life. Status:In Progress Someone is actively working on this now. Type:Enhancement A need for something new.

Comments

@TimDix
Copy link
Contributor

TimDix commented Jul 23, 2015

I'd like to try and improve cache performance by caching blocks and full pages which are currently uncacheable with the current system. Example: Page List with pagination and Form Blocks block full page caching, but could be cached if we took query string variables into account.

I propose that we add support to each block to flag whether or not its contents changes based on query strings. Ideally, we can flag which query strings might affect the contents of this block. We can then use that when checking the full page and block caches to determine what can be cached, and achieve a higher cached results.

I think we also should add additional options to the full page caching to provide control over how to respond to query strings. For example, take a look at cloud flare's options here: https://support.cloudflare.com/hc/en-us/articles/200168256-What-are-CloudFlare-s-caching-levels-

I'd be willing to tackle this myself, but given the far reaching implications, and the fact that it'll be very difficult if not impossible to handle this from within a package, I wanted to get C5 and community thoughts first.

@TimDix
Copy link
Contributor Author

TimDix commented Jul 23, 2015

Example of how this might be implemented. This would signify, that for each unique value of ccm_paging_p, we can cache the block, and consequently, for each unique combination of query strings that we're watching for, cache the full page.

class Controller extends BlockController
{
    protected $btInterfaceHeight = "465";
    protected $btCacheBlockRecord = true;
    protected $btCacheBlockOutput = true;
    protected $btCacheBlockOutputOnPost = true;
    protected $btCacheBlockOutputVariesOnGet = array('ccm_paging_p');
    protected $btSupportsInlineEdit = true;
    protected $btSupportsInlineAdd = true;
    protected $btCacheBlockOutputForRegisteredUsers = false;
    protected $btCacheBlockOutputLifetime = 3600; //until manually updated or cleared

@mitchray
Copy link

+1

@aembler aembler added this to the Future milestone Jul 27, 2015
@aembler
Copy link
Member

aembler commented Jul 27, 2015

Agreed – this would be a nice addition.

@aembler aembler added Type:Enhancement A need for something new. Status:In Progress Someone is actively working on this now. and removed code:core classes labels Dec 22, 2015
@Ruud-Zuiderlicht
Copy link
Contributor

I saw the way this feature request is implemented in #2851. It pre-registers all expected GET parameters and thereby adding arrays that needs to be maintained (and also compiled as list, fetched from URL and checked by the code).

Another way to implement could be the way Contao did it (see contao/core@dd692d6). This way you do not need to pre-register all GET parameters, instead a 404 page is shown when parameters are not used. This way the step of compiling all possible GET parameters is removed, instead only GET parameters that are in the query are looked at. Showing a 404 seems correct to me because incorrect/unused queries are part of a URL to an unknown resource.

The proposed change DOES require that all code uses the Request object to read GET parameters, but that is also an advantage as well because this allows for (semi-forced) centralized sanitation.

@aembler aembler removed this from the Future milestone Mar 5, 2018
@dimger
Copy link
Contributor

dimger commented Aug 24, 2020

Any news here?

@aembler aembler added Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality of Life:3 Enhancements that will have a significant improvement in quality of life. labels Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement:Level Of Effort:3 Enhancements requiring a significant level of effort. Enhancement:Quality of Life:3 Enhancements that will have a significant improvement in quality of life. Status:In Progress Someone is actively working on this now. Type:Enhancement A need for something new.
Projects
None yet
Development

No branches or pull requests

5 participants