Closed (fixed)
Project:
SiteCatalyst
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
9 Apr 2015 at 23:38 UTC
Updated:
16 Sep 2016 at 16:14 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
Crell commentedAnd patch. It should look an awful lot like that the one in the linked issue. :-)
Comment #2
Crell commentedNew version, fixing various bugs.
In my quick benchmarking (on a VM so take with a grain of salt), this caching shaves about 260 ms off of every request. That's quite substantial. That number will of course vary widely depending on the number of token-providing modules installed and their individual cost, and how the module is configured. But as some of those (eg, entity_token) are not known for speed, this does seem worthwhile.
Per the discussion in #2468531: tokens regenerated on every request, very slow the cache key generation is not quite fully reliable yet; we probably need to include entity_changed module as a new dependency. :-(
Edit: Updated time report; as I was off by a decimal. :-)
Comment #3
Crell commentedHere's a version with the dependency added. This should have a more reliable key. I also added clearing of the cache when changing configuration.
Comment #5
Crell commentedAnd without a stupid bug in the key generation. This passes for me locally; let's see if the bot agrees.
Comment #7
Crell commentedThat's fascinating. I don't get anything even remotely like that set of fails locally...
Comment #8
Crell commentedI just tried it on a completely fresh Drupal install, too. Works fine, no errors. Maybe it's related to the new dependency? Is that confusing testbot, perhaps?
Comment #9
Crell commentedRevised version. This now sets the cache item separately per path to avoid the one cache item becoming ginormous. It also adds a config variable to allow the cache to be persisted longer than the next system cron. (It's X seconds, plus the next system cron. X defaults to 0.)
Unfortunately I have no further time on this client project to work on it, and this is working for us locally. So unless it fails for us for some reason I have to bail out here. bleen, hopefully you can carry it on home.
Comment #10
Crell commentedComment #13
dalinSince this is committed, changing to "fixed".