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

malc0mn’s picture

Been 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...

malc0mn’s picture

The 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!

malc0mn’s picture

Getting 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.

malc0mn’s picture

Status: Active » Needs work

Table sorting for summary overview and drush conversion done as well. Only 2 todo's left:

  1. Handle the PERFORMANCE_KEY in drush (makes use of $_SERVER['HTTP_HOST'] which is not defined in php-cli, obviously)
  2. Re-Test migration from 1.x to 2.x, especially when using drush (same as the problem above)
malc0mn’s picture

TODO's in #4 covered.

The 2.x versions are now release ready, but still want to do some small drush improvements:

  1. when no results found for drush perf-sm, inform the user of the possible causes:
    • no data stored
    • data too old (perf-sm only goes back one hour)
    • $base_url not properly set (run with --uri parameter)
  2. add the running averages as in the drupal backend pages (come to think of it, they might be wrong now in the backend pages!)
malc0mn’s picture

Status: Needs work » Fixed

TODO's in #5 covered together with some small improvements.

Ready to create a release now.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.