Re: [PATCH] mod_cache fixes: #8

2004-08-03 Thread Brian Akins
Bill Stoddard wrote: Please, no more specialized knobs which 99.9% of the world cares nothing about. How do you define that percentage? By domains? In that case 99.999% probably care nothing about what we are doing. If you look at total traffic, however, would not options that help the

Re: [PATCH] mod_cache fixes: #6

2004-08-03 Thread Ian Holsman
Justin Erenkrantz wrote: --On Monday, August 2, 2004 11:44 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: These are debug messages so not sure why they are a problem. +0 The logging code is expensive to call for every request like that as many times as it does. IMHO, there's no benefit to

[PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Some more work, analysis, and tests yielded apr_file_gets() and MD5 as two more bottlenecks. I've

Re: [PATCH] mod_cache fixes

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: 3) Allow mod_disk_cache to use sendfile This, in my experience is a huge gain. Another thing to consider is to use normal open/read/close on the headers file. This saves a few apr_alloc's and strdup's, which in turn saves some mutex locks. This may not seem like

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: - static void mkdir_structure(disk_cache_conf *conf, char *file, apr_pool_t *pool) Another big speed-up may be to pre-make all of the directories. A simple script could use CacheRoot, |CacheDirLength|, and |CacheDirLevels to create them all. Just require that this

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Jeff Trawick
On Mon, 02 Aug 2004 08:46:07 -0400, Brian Akins [EMAIL PROTECTED] wrote: Justin Erenkrantz wrote: - static void mkdir_structure(disk_cache_conf *conf, char *file, apr_pool_t *pool) Another big speed-up may be to pre-make all of the directories. A simple script could use CacheRoot,

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Jeff Trawick wrote: Wouldn't assuming that the directory is already there be sufficient (and then create the directory structure on the error path)? It looks to me that we only assume the directory structure exists if we had this very same file cached previously. In a low traffic site, yes.

Re: [PATCH] mod_cache fixes: #1

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/http/http_request.c (ap_internal_redirect): Call quick_handler

Re: [PATCH] mod_cache fixes: #2

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.h: Always use atomics. *

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_disk_cache.c: Allow sendfile on cache bodies.

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Brian Akins
Bill Stoddard wrote: /* Open the headers file */ -rc = apr_file_open(hfd, headers, APR_READ|APR_BINARY, 0, r-pool); +rc = apr_file_open(hfd, headers, flags, 0, r-pool); Should be something like this adapted from core: core_dir_config *core_config; core_config = (core_dir_config *)

Re: [PATCH] mod_cache fixes: #4

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. (This is probably the largest and most complicated one. At the bottom,

Re: [PATCH] mod_cache fixes: #5

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.c: Delay no-store check until saving.

Re: [PATCH] mod_cache fixes: #6

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.c: Reduce logging in mainline case. These

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Bill Stoddard
Brian Akins wrote: Bill Stoddard wrote: /* Open the headers file */ -rc = apr_file_open(hfd, headers, APR_READ|APR_BINARY, 0, r-pool); +rc = apr_file_open(hfd, headers, flags, 0, r-pool); Should be something like this adapted from core: core_dir_config *core_config; core_config =

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread William A. Rowe, Jr.
At 10:46 AM 8/2/2004, Bill Stoddard wrote: EnableSendfile off is a global directive (?) so only need to check it once at startup and save it in a static variable? Oh, no! Directory /fs1/shared/www EnableSendfile Off /Directory kills sendfile for that mount. Bill

Re: [PATCH] mod_cache fixes: #6

2004-08-02 Thread Graham Leggett
Bill Stoddard wrote: * modules/experimental/mod_cache.c: Reduce logging in mainline case. These are debug messages so not sure why they are a problem. While mod_cache is experimental, it may help to have more logging rather than less. Are the logging functions that much of a performance problem

Re: [PATCH] mod_cache fixes: #7

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_disk_cache.c (load_headers): Only validate

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Brian Akins
William A. Rowe, Jr. wrote: Oh, no! Directory /fs1/shared/www EnableSendfile Off /Directory kills sendfile for that mount. Bill However, in a quick handler, only the global config matters. Am I correct? Because the per_dir has not been merged until map to storage and Location is later.

Re: [PATCH] mod_cache fixes: #6

2004-08-02 Thread Jeff Trawick
On Mon, 02 Aug 2004 18:13:09 +0200, Graham Leggett [EMAIL PROTECTED] wrote: Bill Stoddard wrote: * modules/experimental/mod_cache.c: Reduce logging in mainline case. These are debug messages so not sure why they are a problem. While mod_cache is experimental, it may help to have more

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 10:35 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: * modules/experimental/mod_disk_cache.c: Allow sendfile on cache bodies. -1, Need to check for EnableSendfile off. No, core_output_filter does that check. Modules don't have that information whether

Re: [PATCH] mod_cache fixes: #6

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 11:44 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: These are debug messages so not sure why they are a problem. +0 The logging code is expensive to call for every request like that as many times as it does. IMHO, there's no benefit to such a verbose log. More

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Monday, August 2, 2004 10:35 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: * modules/experimental/mod_disk_cache.c: Allow sendfile on cache bodies. -1, Need to check for EnableSendfile off. No, core_output_filter does that check. Modules don't have that

Re: [PATCH] mod_cache fixes

2004-08-02 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Thanks for the review! I've committed all of the non-contentious ones (i.e. sendfile and logging).

[PATCH] mod_cache fixes: #9

2004-08-02 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Here's another patch that was hidden in my earlier one. I think 'read' and 'write' are awful terms

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 1:05 PM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: I should amend my vote a -.5. The patch should work as you've coded it but opening a file for use with apr_sendfile causes the file to be opened for overlapped i/o on Windows. I expect some of the codepaths will

Re: [PATCH] mod_cache fixes: #3

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Monday, August 2, 2004 1:05 PM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: I should amend my vote a -.5. The patch should work as you've coded it but opening a file for use with apr_sendfile causes the file to be opened for overlapped i/o on Windows. I expect

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 8:46 AM -0400 Brian Akins [EMAIL PROTECTED] wrote: Another big speed-up may be to pre-make all of the directories. A simple script could use CacheRoot, |CacheDirLength|, and |CacheDirLevels to create them all. Just require that this script be ran before starting a

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 9:21 AM -0400 Brian Akins [EMAIL PROTECTED] wrote: In a low traffic site, yes. In a very high traffic site, with lots of objects, the mkdir's kill you. After a while, most of the directories will be created. However, bringing up a fresh server behind a very busy

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: post_config's not a bad place for that. But, I've yet to get a good handle on my thoughts for the storage mechanism that mod_disk_cache is using. My hunch so far is that it's really inefficient. (The reading of the headers one-byte at a time with the brain-dead

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 10:55 AM -0700 Justin Erenkrantz [EMAIL PROTECTED] wrote: --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Here's another

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: mod_cache isn't doing anything stupid or damaging performance-wise, I'd like to start being more aggressive about what we can cache. From my perspective, these patches I've posted (and started to commit) are just the beginning of trying to get mod_cache on more solid

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: The other bottleneck I looked at was MD5 as the on-disk naming scheme. I think MD5 is a poor choice here because it's not very fast. Ideally, switching to a variant of the times-33 hash might work out better. *shrug* How to handle collisions? -- Brian Akins Senior

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Bill Stoddard wrote: To get mod_cache/mod_mem_cache (I know little or nothing about mod_disk_cache) really performing competatively against best-of-breed caches will require bypassing output filters Yes. (and prebuilding headers) Is there a way to send pre built headers? mod_asis uses

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 2:54 PM -0400 Brian Akins [EMAIL PROTECTED] wrote: The other bottleneck I looked at was MD5 as the on-disk naming scheme. I think MD5 is a poor choice here because it's not very fast. Ideally, switching to a variant of the times-33 hash might work out better.

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 3:01 PM -0400 Brian Akins [EMAIL PROTECTED] wrote: (and prebuilding headers) Is there a way to send pre built headers? mod_asis uses ap_scan_script_header_er which is fairly slow. That was what I fixed with apr_file_gets() last night. The code there was really

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 2:49 PM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: To get mod_cache/mod_mem_cache (I know little or nothing about mod_disk_cache) really performing competatively against best-of-breed caches will require bypassing output filters (and prebuilding headers) and

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: MD5 has the possibility for collisions, too. What do squid or other proxies do? True. I'll see what others do. On one hand, I think doing MD5 is sort of silly - just use the URL itself. *shrug* -- justin With mod_disk_cache, how about urls such as:

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Brian Akins
Justin Erenkrantz wrote: MD5 has the possibility for collisions, too. Any reason md4 was not used in mod_cache? In my ad hoc tests, it seems much faster. I do not know the in-and-outs of encryption, but is there any compelling reason to use md5 over md4 in this case? We don't care if

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Mads Toftum
On Mon, Aug 02, 2004 at 04:54:07PM -0400, Brian Akins wrote: Any reason md4 was not used in mod_cache? In my ad hoc tests, it seems much faster. I do not know the in-and-outs of encryption, but is there any compelling reason to use md5 over md4 in this case? We don't care if someone

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Graham Leggett
Brian Akins wrote: On one hand, I think doing MD5 is sort of silly - just use the URL itself. *shrug* -- justin One of the goals of the new mod_cache was to support the caching of URL variants. I actually have somewhat of a solution: URL encode the uri and any vary elements:

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Bill Stoddard
Mads Toftum wrote: On Mon, Aug 02, 2004 at 04:54:07PM -0400, Brian Akins wrote: Any reason md4 was not used in mod_cache? In my ad hoc tests, it seems much faster. I do not know the in-and-outs of encryption, but is there any compelling reason to use md5 over md4 in this case? We don't care

Re: [PATCH] mod_cache fixes: #8

2004-08-02 Thread Graham Leggett
Bill Stoddard wrote: To get mod_cache/mod_mem_cache (I know little or nothing about mod_disk_cache) really performing competatively against best-of-breed caches will require bypassing output filters (and prebuilding headers) and possibly bypassing or at least reworking input filters. The

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Roy T . Fielding
On Monday, August 2, 2004, at 10:55 AM, Justin Erenkrantz wrote: Avoid confusion when reading mod_cache code. write_ and read_ often imply network code; save_ and load_ are more understandable prefixes in this context. Hmm, IIRC, loading a cache means writing to it, not reading from it. Why

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Cliff Woolley
On Mon, 2 Aug 2004, Roy T.Fielding wrote: Hmm, IIRC, loading a cache means writing to it, not reading from it. Doh. That does ring a bell in the back of my (usually lousy) memory. :) Why not just change them to cache_write and cache_read? Or store and recall? store and recall seem

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 5:11 PM -0700 Roy T. Fielding [EMAIL PROTECTED] wrote: Hmm, IIRC, loading a cache means writing to it, not reading from it. Why not just change them to cache_write and cache_read? Or store and recall? store and recall work, too. *shrug* (I share rbb's inability to

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Justin Erenkrantz
--On Monday, August 2, 2004 8:18 PM -0400 Cliff Woolley [EMAIL PROTECTED] wrote: Yeah, it's really good to see the interest in this picking back up. This seems like a really good way to get us motivated to do a 2.2 release Sometime Soon. :) How 'bout 2.2 GA for ApacheCon? Seems reasonable to

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Bill Stoddard
Justin Erenkrantz wrote: --On Monday, August 2, 2004 8:18 PM -0400 Cliff Woolley [EMAIL PROTECTED] wrote: Yeah, it's really good to see the interest in this picking back up. This seems like a really good way to get us motivated to do a 2.2 release Sometime Soon. :) How 'bout 2.2 GA for

Re: [PATCH] mod_cache fixes: #9

2004-08-02 Thread Cliff Woolley
On Mon, 2 Aug 2004, Bill Stoddard wrote: How 'bout 2.2 GA for ApacheCon? Seems reasonable to me. ;-) -- justin +1, sounds like a good target to shoot for. +1

[PATCH] mod_cache fixes

2004-08-01 Thread Justin Erenkrantz
While at OSCON last week, Madhu and I had a chat about mod_cache - and how Zeus is beating httpd 2.x all over the place because they cache and we still don't by default and about how poor a job mod_cache is doing in the real world. Madhu has a bunch of gprof numbers that he said he'll be posting

Re: [PATCH] mod_cache fixes

2004-08-01 Thread Bill Stoddard
Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Bill

Re: [PATCH] mod_cache fixes

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. Okay. You asked for it. ;-) I wanted to let the patches sit overnight in my head and then break

[PATCH] mod_cache fixes: #1

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/http/http_request.c (ap_internal_redirect): Call quick_handler when we do an internal

[PATCH] mod_cache fixes: #2

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.h: Always use atomics. * modules/experimental/mod_mem_cache.c:

[PATCH] mod_cache fixes: #3

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_disk_cache.c: Allow sendfile on cache bodies. Index:

[PATCH] mod_cache fixes: #5

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.c: Delay no-store check until saving. (It's a corner case with

[PATCH] mod_cache fixes: #6

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_cache.c: Reduce logging in mainline case. Index:

[PATCH] mod_cache fixes: #7

2004-08-01 Thread Justin Erenkrantz
--On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard [EMAIL PROTECTED] wrote: Too many changes in one patch. Break this up into multiple consumable in 15 minute patches and I'll review them. * modules/experimental/mod_disk_cache.c (load_headers): Only validate that the header file