Digging into the details of this module, I really don't think we need to have multiple logging modes.
Performance summary logging is basically just logging some basic information to a cache store, and retrieving it for updating or display.
If we simply log everything using Drupal's native cache functions to a specific cache table (pretty much like the memcache implementation already does), then performance logging would work with whatever caching backend the user had configured. This might be database, it might be memcache, or it might be APC, memcache, file, ... via cacherouter, or some entirely different system.
This would vastly simplify the management of this module, as well as make it compatible with other caching systems (such as cacherouter) without requiring any additional effort.
The downsides I can see are that it would make it impossible to use a different system for performance logging than the general cache (although this could still be done by using cacherouter on the backend) - although why one would want to do that, I'm not sure. Also, when logging to the database, directly logging to a table is probably slightly more efficient than logging to the database via the cache system, but no serious person using this module is probably logging to the database in production anyway, so I don't see this as a major issue.
Thoughts?
Comments
Comment #1
malc0mn CreditAttribution: malc0mn commentedBeen thinking about changing the implementation of cache mechanisms as well. The current beta releases have this refactored for easy maintenance and a bit of performance gain but using the basic cache approach is indeed not a bad idea at all. I would propose to make a cache_performance table for this purpose which can then be mapped to any cache mechanism you please.
This would be a good approach for a 2.0 release I think. The most major downside I see for this is the currently existing problem of tablesort and paging. In the current version, this is only working when logging to DB. Using cache_get and cache_set will also introduce this problem when no apc and/or memcache is available as all data will be serialised...
If there is a good solution to cover this (still have not found one), then I'm all for this by way of a 2.0 release.
I'm not sure about cache router though. Used in the past for D6 but was never really happy with the result and there is, rightfully so, no D7 support.
These will do fine though:
http://drupal.org/project/apc
http://drupal.org/project/memcache
Edit: http://drupal.org/project/filecache
and as the D7 caching can very easily be expanded as these modules show a zendcache module could easily be derived from the APC module...
Comment #2
malc0mn CreditAttribution: malc0mn commentedThe 7.x-2.x and 6.x-2.x branches have a working implementation of this idea: only using cache_get and cache_set for summary logging now, cool :-)
Still todo is table sorting for the summary page (and a lot of other stuff :p), found the right direction for adding that functionality!
Comment #3
malc0mn CreditAttribution: malc0mn commentedGetting there... Most important stuff to do is convert the drush commands to handle the new cache_performance bin + add table sorting to the performance summary overview (found a good approach which should work just fine).
Feel free to checkout the 7.x-2.x or 6.x-2.x branches to test.
Comment #4
malc0mn CreditAttribution: malc0mn commentedTable sorting for summary overview and drush conversion done as well. Only 2 todo's left:
Comment #5
malc0mn CreditAttribution: malc0mn commentedTODO's in #4 covered.
The 2.x versions are now release ready, but still want to do some small drush improvements:
drush perf-sm
, inform the user of the possible causes:$base_url
not properly set (run with--uri
parameter)Comment #6
malc0mn CreditAttribution: malc0mn commentedTODO's in #5 covered together with some small improvements.
Ready to create a release now.