#2468531: tokens regenerated on every request, very slow introduced tag caching, but the implementation is broken for sites that use tokens to vary by base root, query string parameter, theme, language.
While the blocks are cached per page, the tags are cached per system path.
My use case is with the current-domain token, but perhaps a home page view that varies by language is a simple enough case.
To solve this we should either cache per page, or make this opt in. (Setting a one second lifetime is not good enough.)
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | dfp-cache-2579259-14.patch | 1.29 KB | eric_a |
| #11 | dfp-cache-2579259-11.patch | 534 bytes | eric_a |
| #10 | dfp-cache-2579259-10.patch | 534 bytes | eric_a |
| #4 | dfp-cache-2579259-3.patch | 565 bytes | eric_a |
Comments
Comment #2
eric_a commentedComment #3
eric_a commentedComment #4
eric_a commentedHere's an initial patch to get things going. It seems though as dfp uses a hardcoded list of data objects for token_replace(). What happens if a token is entered that needs a different object, like for example a current-domain token?
Comment #5
eric_a commentedComment #6
eric_a commentedComment #7
eric_a commentedPatch works for our token.
Tokens with query parameters or pagination or absolute URL with language prefix are all broken, as far as I can see. Changing priority to critical.
Comment #8
bleen commentedCan you include a test to show a failing case?
Comment #9
eric_a commentedIt seems that the current test classes have no dependencies. Without dependencies on token (or domain) it's pretty hard to come up with a decent test scenario.
Which required module provides a token that varies by theme, base root, language prefix? None, I guess.
Not sure how to proceed...
Comment #10
eric_a commentedHere's an alternative patch that shows a fix for my Domain Access use case, with a much better cache hit ratio than the generic patch in #4.
Comment #11
eric_a commentedMoved the "/ ", so that sites without domain keep the same cache ID.
Comment #12
bleen commentedThe patch in #10actually made the patch in #4 make much more sense to me ... thanks. I'm going to go ahead and commit #4 since it works in a more general case.
Comment #14
eric_a commentedEDIT: Sigh, and please ignore the accidental garbage in the test file.
Thanks!
For the record: the patches in #10 and #11 are broken, for domain_get_domain() returns an array. See attached patch for a fix. (Implementation for recent versions of PHP, obviously).