Path Caching

tomchuk - August 11, 2008 - 14:40
Project:Advanced cache
Version:5.x-1.9
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review
Description

I miss path caching :(

Here's a couple of patches that are doing the trick for me, been running in production for a week or so on a 2M+ pageview/month site. It's nice to see those drupal_lookup_path's disappear from the devel query logs again. The path.inc.no_serialize.patch is for memcached users.

AttachmentSize
advcache.patch428 bytes
path.inc.patch2.49 KB
path.inc_no_serialize.patch2.44 KB

#1

tomchuk - August 11, 2008 - 16:42

Forgot to mention, path.inc patches are against Drupal 5.9

#2

firebus - September 25, 2008 - 01:41

tomchuck, do these patches resolve the issues raised in http://drupal.org/node/220983? if so, how?

#3

firebus - September 25, 2008 - 02:11

also, please make sure to apply jeremy's change from http://drupal.org/node/230235

#4

tomchuk - October 7, 2008 - 17:58

When imagecache handles a request it calls file_transfer() at the end of its menu callback for file_directory_path() .'/imagecache' (the imagecache_cache function). file_transfer() calls exit() after sending the image, which means that hook_exit is never called. Because the caching is done in hook_exit (see advcache.patch), the no_src and map are never cached for imagecache requests. I have confirmed this with watchdogs in advcache_exit and 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.

#5

tomchuk - September 30, 2008 - 21:40

I 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() after file_transfer(). Imagecache 2.1 changes its behavior quite a bit, but every path out of _imagecache_cache that I can see ends in a call to exit(), which means that this patch should work fine with all 5.x releases of imagecache.

#6

tomchuk - September 30, 2008 - 21:49

Here's the above patches with jeremy's modified count query.

AttachmentSize
path.inc.patch 2.63 KB
path.inc_no_serialize.patch 2.58 KB

#7

tomchuk - September 30, 2008 - 22:11

Wow 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.13s
SELECT COUNT(pid) FROM url_alias; - MyISAM: 0.081s | InnoDB: 59.39s
SELECT pid FROM url_alias LIMIT 1; - MyISAM: 0.078s | InnoDB: 0.094s

#8

tomchuk - October 20, 2008 - 14:30

Running imagecache 2.1 now in production, and things are still working great.

#9

iKillBill2 - November 2, 2008 - 05:06

will this work on drupal 5.12?

#10

tomchuk - November 3, 2008 - 22:00

Yes, path.inc and advcache.module patches apply with a little fuzz on Drupal 5.12 and Advcache 1.9.

#11

petrupsihologu - November 16, 2008 - 21:15

I'm using it on a Drupal 5.12 production site and it looks OK. No problems reported.
Thx for the patches.

#12

mikejoconnor - December 9, 2008 - 02:05
Status:needs review» needs work

I'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?

#13

mikejoconnor - December 9, 2008 - 03:15

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

AttachmentSize
d6-6.path_.patch 2.84 KB

#14

mikejoconnor - December 9, 2008 - 03:19
Status:needs work» needs review

sorry, didn't mean to change the status earlier

#15

Brumisek - November 3, 2009 - 18:56
Version:5.x-1.x-dev» 5.x-1.9

What about patch for memacache module? I also need path (url_alias) improved in this module :(:( Drupal 5.x

 
 

Drupal is a registered trademark of Dries Buytaert.