Needs review
Project:
Advanced cache
Version:
5.x-1.9
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
11 Aug 2008 at 14:40 UTC
Updated:
3 Nov 2009 at 18:56 UTC
Jump to comment: Most recent file
Comments
Comment #1
tomchuk commentedForgot to mention, path.inc patches are against Drupal 5.9
Comment #2
firebus commentedtomchuck, do these patches resolve the issues raised in http://drupal.org/node/220983? if so, how?
Comment #3
firebus commentedalso, please make sure to apply jeremy's change from http://drupal.org/node/230235
Comment #4
tomchuk commentedWhen imagecache handles a request it calls
file_transfer()at the end of its menu callback forfile_directory_path() .'/imagecache'(theimagecache_cachefunction).file_transfer()callsexit()after sending the image, which means thathook_exitis never called. Because the caching is done inhook_exit(see advcache.patch), the no_src and map are never cached for imagecache requests. I have confirmed this with watchdogs inadvcache_exitand with my own cache_path hit rate (low 80s) on a highly trafficed site with very heavy use of imagecache (30+ presets and 10k+ images)Regarding the count query, I didn't think it appropriate to make any more changes than necessary from core. With my path.inc patch applied, the query remains the same as the one in core. If there are dramatic gains to be had by tweaking the query, I think it should be submitted as a core patch, not addressed in a contrib module. Just my $0.02.
Comment #5
tomchuk commentedI should also note that the above is for imagecache-5.x-1.5. I have downloaded 1.6 and the code is the same, but ads another call to
exit()afterfile_transfer(). Imagecache 2.1 changes its behavior quite a bit, but every path out of_imagecache_cachethat I can see ends in a call toexit(), which means that this patch should work fine with all 5.x releases of imagecache.Comment #6
tomchuk commentedHere's the above patches with jeremy's modified count query.
Comment #7
tomchuk commentedWow that does make a difference on InnoDB, with no real difference on MyISAM.
Execution times for 1000 queries with no query caching, average of 3 runs on url_alias table with 160155 entries.
SELECT COUNT(*) FROM url_alias;- MyISAM: 0.071s | InnoDB: 57.13sSELECT COUNT(pid) FROM url_alias;- MyISAM: 0.081s | InnoDB: 59.39sSELECT pid FROM url_alias LIMIT 1;- MyISAM: 0.078s | InnoDB: 0.094sComment #8
tomchuk commentedRunning imagecache 2.1 now in production, and things are still working great.
Comment #9
iKillBill2 commentedwill this work on drupal 5.12?
Comment #10
tomchuk commentedYes, path.inc and advcache.module patches apply with a little fuzz on Drupal 5.12 and Advcache 1.9.
Comment #11
Petrica commentedI'm using it on a Drupal 5.12 production site and it looks OK. No problems reported.
Thx for the patches.
Comment #12
mikejoconnor commentedI've been running this on a site with 5m page views/month, with 1.2 million url aliases. Its made a HUGE difference. What further testing do we need to bring this back to the main advcache module?
Comment #13
mikejoconnor commentedHere is a patch for this version of the path fix, for D6. For the rest of the d6 upgrade please see Issue 242121.
Seems to be working well for me.
Comment #14
mikejoconnor commentedsorry, didn't mean to change the status earlier
Comment #15
Brumisek commentedWhat about patch for memacache module? I also need path (url_alias) improved in this module :(:( Drupal 5.x