You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brian Pane <bp...@pacbell.net> on 2001/09/10 04:46:30 UTC

[PATCH] performance patch for mod_log_config

The call to apr_explode_localtime() in mod_log_config is one of the more
expensive operations in the httpd.  This patch attempts to reduce the
overhead by caching the result.

--Brian


Index: modules/loggers/mod_log_config.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/loggers/mod_log_config.c,v
retrieving revision 1.68
diff -r1.68 mod_log_config.c
447a448,498
 >
 > /* Cache of expanded timestamps:
 >  *  Because apr_explode_localtime is an expensive function call,
 >  *  we cache the exploded time and re-use it for the rest of the
 >  *  current second.
 >  */
 >
 > struct exploded_time_cache_element {
 >     apr_int64_t t;
 >     apr_exploded_time_t xt;
 > };
 >
 > #define TIME_CACHE_SIZE 16
 > static struct exploded_time_cache_element 
exploded_time_cache[TIME_CACHE_SIZE];
 >
 > static void cached_explode_localtime(apr_exploded_time_t *xt, 
apr_time_t t)
 > {
 >     apr_int64_t seconds = t / APR_USEC_PER_SEC;
 >     struct exploded_time_cache_element *cache =
 >         &(exploded_time_cache[seconds % TIME_CACHE_SIZE]);
 >
 >     /* The cache is implemented as a ring buffer.  Each second,
 >      * it uses a different element in the buffer.  The timestamp
 >      * in the element indicates whether the element contains the
 >      * exploded time for the current second (vs the time
 >      * 'now - TIME_CACHE_SIZE' seconds ago).  If the cached
 >      * value is for the current time, we use it.  Otherwise,
 >      * we compute the apr_exploded_time_t and store it in this
 >      * cache element. Note that the timestamp in the cache
 >      * element is updated only after the exploded time.  Thus
 >      * if two threads hit this cache element simultaneously
 >      * at the start of a new second, they'll both explode the
 >      * time and store it.  I.e., the writers will collide, but
 >      * they'll be writing the same value.
 >      */
 >     if (cache->t >= seconds) {
 >         /* Note: If this memcpy ever takes more than TIME_CACHE_SIZE
 >          * seconds, the value will be unpredictable (because this
 >          * bucket in the ring buffer will have been recycled).
 >          * This memcpy should never take multiple seconds, but to
 >          * be safe we use a relatively large value for TIME_CACHE_SIZE.
 >          */
 >         memcpy(xt, &(cache->xt), sizeof(apr_exploded_time_t));
 >     }
 >     else {
 >         apr_explode_localtime(xt, t);
 >         memcpy(&(cache->xt), xt, sizeof(apr_exploded_time_t));
 >         cache->t = seconds;
 >     }
 > }
 >
466c517
<     apr_explode_localtime(&xt, apr_time_now());
---
 >     cached_explode_localtime(&xt, apr_time_now());
468c519
<     apr_explode_localtime(&xt, r->request_time);
---
 >     cached_explode_localtime(&xt, r->request_time);








Re: [PATCH] performance patch for mod_log_config

Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:

>On Sunday 09 September 2001 19:46, Brian Pane wrote:
>
>Can we get this as a unified diff?
>
sure, here's the unified form:

Index: modules/loggers/mod_log_config.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/loggers/mod_log_config.c,v
retrieving revision 1.68
diff -u -r1.68 mod_log_config.c
--- modules/loggers/mod_log_config.c    2001/08/27 20:50:01    1.68
+++ modules/loggers/mod_log_config.c    2001/09/10 04:17:59
@@ -445,6 +445,58 @@
     return NULL;
 }
 
+
+/* Cache of expanded timestamps:
+ *  Because apr_explode_localtime is an expensive function call,
+ *  we cache the exploded time and re-use it for the rest of the
+ *  current second.
+ */
+
+struct exploded_time_cache_element {
+    apr_int64_t t;
+    apr_exploded_time_t xt;
+};
+
+#define TIME_CACHE_SIZE 16
+static struct exploded_time_cache_element 
exploded_time_cache[TIME_CACHE_SIZE];
+
+static void cached_explode_localtime(apr_exploded_time_t *xt, apr_time_t t)
+{
+    apr_int64_t seconds = t / APR_USEC_PER_SEC;
+    struct exploded_time_cache_element *cache =
+        &(exploded_time_cache[seconds % TIME_CACHE_SIZE]);
+
+    /* The cache is implemented as a ring buffer.  Each second,
+     * it uses a different element in the buffer.  The timestamp
+     * in the element indicates whether the element contains the
+     * exploded time for the current second (vs the time
+     * 'now - TIME_CACHE_SIZE' seconds ago).  If the cached
+     * value is for the current time, we use it.  Otherwise,
+     * we compute the apr_exploded_time_t and store it in this
+     * cache element. Note that the timestamp in the cache
+     * element is updated only after the exploded time.  Thus
+     * if two threads hit this cache element simultaneously
+     * at the start of a new second, they'll both explode the
+     * time and store it.  I.e., the writers will collide, but
+     * they'll be writing the same value.
+     */
+    if (cache->t >= seconds) {
+        /* Note: If this memcpy ever takes more than TIME_CACHE_SIZE
+         * seconds, the value will be unpredictable (because this
+         * bucket in the ring buffer will have been recycled).
+         * This memcpy should never take multiple seconds, but
+         * TIME_CACHE_SIZE has a relatively large value just in
+         * case.
+         */
+        memcpy(xt, &(cache->xt), sizeof(apr_exploded_time_t));
+    }
+    else {
+        apr_explode_localtime(xt, t);
+        memcpy(&(cache->xt), xt, sizeof(apr_exploded_time_t));
+        cache->t = seconds;
+    }
+}
+
 static const char *log_request_time(request_rec *r, char *a)
 {
     apr_exploded_time_t xt;
@@ -463,9 +515,9 @@
     a problem with this, you can set the define.  -djg
     */
 #ifdef I_INSIST_ON_EXTRA_CYCLES_FOR_CLF_COMPLIANCE
-    apr_explode_localtime(&xt, apr_time_now());
+    cached_explode_localtime(&xt, apr_time_now());
 #else
-    apr_explode_localtime(&xt, r->request_time);
+    cached_explode_localtime(&xt, r->request_time);
 #endif
     if (a && *a) {              /* Custom format */
         apr_strftime(tstr, &retcode, MAX_STRING_LEN, a, &xt);



Re: [PATCH] performance patch for mod_log_config

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 09 September 2001 19:46, Brian Pane wrote:

Can we get this as a unified diff?

Ryan

> The call to apr_explode_localtime() in mod_log_config is one of the more
> expensive operations in the httpd.  This patch attempts to reduce the
> overhead by caching the result.

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [PATCH] performance patch for mod_log_config

Posted by Alex Stewart <al...@foogod.com>.
Brian Pane wrote:
> William A. Rowe, Jr. wrote:
> 
>> Is this the right place to be caching, or should this become a 
>> straightforward
>> optimization to apr's time.c functions?  I'd think the advantages are 
>> many for
>> keeping 15 current seconds in apr, and would pay off across the 
>> board.  Within
>> apr, we can always recalculate just the ms as well, for fun.
>>
> I think putting it in APR would work.  The one limitation I can think of
> is that adding the cache in apr_explode_localtime() itself wouldn't be a
> win because we'd have to add the overhead of a gettimeofday() call to
> check whether the supplied time was indeed current (and thus susceptible
> to caching).

I'm not that familiar with the usage patterns of this function in Apache 
et al, but it seems to me that a gettimeofday() call could be avoided 
reasonably well by simply tracking the previously-used time argument. 
If the current call is for a time which is later than the one of the 
last call (within a certain margin), cache it.  This should be just 
about as effective as checking gettimeofday() (possibly moreso, as it 
would also match anything within a reasonable proximity of the current 
time).  It might miss one or two if there are long periods without a 
call, but in that situation it's arguable that the speed of this 
function probably isn't the largest concern anyway.

Just a thought..

-alex


Re: [PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 09 September 2001 23:51, Brian Pane wrote:
> Cliff Woolley wrote:
> >On Sun, 9 Sep 2001, Brian Pane wrote:
> >>I think putting it in APR would work.  The one limitation I can think of
> >>is that adding the cache in apr_explode_localtime() itself wouldn't be a
> >>win because we'd have to add the overhead of a gettimeofday() call to
> >>check whether the supplied time was indeed current (and thus susceptible
> >>to caching).
> >
> >It took me a minute to figure out why this was a problem, but you're
> >right.  I don't really like the idea of combining time_now and
> >explode_localtime, though.  What about some kind of deal like
> >apr_explode_recent_localtime() which is the same as
> >apr_explode_localtime() except that the input must never be older than
> >TIME_CACHE_SIZE seconds?  Or is that too httpd-tailored?  If so, no sweat,
> >make it an ap_ function instead of an apr_ function.
>
> I think it's relatively httpd-specific, so I used
> ap_explode_recent_localtime().
> Here's the revised patch, plus the new files include/util_time.h and
> server/util_time.c

+1

Ryan
______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

[...]

>but still, does this really work?  'cause the time used is
>r->request_time, if a request takes longer than 14 seconds or so to
>generate/serve then the logger is going to be asking for timestamps which
>are dangerously close to your cache rollover.   and since you can't
>update a cache entry atomically it's possible they'll see a partially
>updated entry, and get bogus results.
>
You're right; a request that takes almost exactly 15 seconds is
vulnerable to corrupt data.  I have a fix, though, that doesn't
require either locking or a gettimeofday() call.  I'll post an
updated patch once I finish testing it.

--Brian



Re: [PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by dean gaudet <de...@arctic.org>.
On Mon, 10 Sep 2001, Brian Pane wrote:

> dean gaudet wrote:
>
> > why is there a need for 15 entries?  if it's a multiprocess server then
> > there's only a need for 1 or 2 entries.  if it's a multithreaded server
> > then you need to lock the cache (which you're not doing :)
>
> The whole point of the design is to do the multithreaded cache
> without locks.

oh i see.  i think you should carefully comment this then, 'cause this
is really subtle... the fact that you copy into the cache from distinct
apr_exploded_time_t * is what makes it safe.  (i.e. take a look at the
win32 time.c code and notice there's a few ++ operations which would
not be multithreaded safe.)

but still, does this really work?  'cause the time used is
r->request_time, if a request takes longer than 14 seconds or so to
generate/serve then the logger is going to be asking for timestamps which
are dangerously close to your cache rollover.   and since you can't
update a cache entry atomically it's possible they'll see a partially
updated entry, and get bogus results.

> > isn't the real win in eliminating both the divisions required by the
> > explode time functions and the divisions required by the printf?
>
> That's what I used to think, but the profile data showed that localtime()
> (which does most of the work in apr_explode_localtime()) is an order of
> magnitude more expensive than the apr_snprintf() that formats the resulting
> data.  Caching or otherwise optimizing the printf in the future wouldn't
> hurt, but it's a more marginal win than caching the exploded time struct.
>
> > also -- you could eliminate the seconds = t / APR_USECS_PER_SEC
> > calculation (death to all divisions!  especially the 64-bit ones)...
> > because this division already happens inside the explode function:
> >
> > when you miss the cache and do a full explode, then subtract the
> > resulting
> > tm_usec off the input time and you've got an apr_time_t which lands
> > exactly on a second boundary.  use that as your cache key, and you can
> > then do ranged comparisons (t >= cache->t && t < cache->t +
> > APR_USECS_PER_SEC).
>
> The one problem I can see with this is that removing the division means
> that you have to scan through the table to find out whether you have a
> cache hit or not--which probably negates the performance benefit of
> removing the division.

hehe, try this:

% cat >t.c <<EOF
long long r(long long t)
{
	return t / 1000000LL;
}

int main(int argc, char **argv)
{
	return 0;
}
EOF
% gcc -O2 -g t.c
% gdb a.out
(gdb) disass r
(gdb) disass __divdi3

(__divdi3 is what you should see a call to in r ... assuming you're on
a 32-bit box.)  64-bit division is a really big out of line subroutine :)

that's why strength reductions like i was suggesting are a good idea.

i hadn't looked at your code close enough to see that you didn't already
scan the cache ... cache scanning isn't SMP safe either...

btw, TIME_CACHE_SIZE should be a power of 2 so that the cache hash can
be strength reduced to a binary-and :)

-dean


Re: [PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

> why is there a need for 15 entries?  if it's a multiprocess server then
> there's only a need for 1 or 2 entries.  if it's a multithreaded server
> then you need to lock the cache (which you're not doing :) 

The whole point of the design is to do the multithreaded cache
without locks.  Instead of trying to prevent a race condition,
the cache is designed to make the race condition safe: when two
readers or a reader and a writer collide, the threads end up
computing the exploded time redundantly.  The race condition
that's not safe, though, is when one thread tries to explode
a time_t representing 'now - num_entries' seconds ago at the
same time that another thread is updating that same cache
element with the exploded value for the current time (the two
timestamps map to the same element in the ring buffer).  The
number of entries in the ring buffer thus determines how old
an input timestamp the cache can support and still provide
deterministic results: I used the 15-second value to provide
a large safety factor.

> isn't the real win in eliminating both the divisions required by the
> explode time functions and the divisions required by the printf?

That's what I used to think, but the profile data showed that localtime()
(which does most of the work in apr_explode_localtime()) is an order of
magnitude more expensive than the apr_snprintf() that formats the resulting
data.  Caching or otherwise optimizing the printf in the future wouldn't
hurt, but it's a more marginal win than caching the exploded time struct.

>   fwiw,
> tux does binary logging, and has an external tool to convert it to CLF...
> and the spec committee accepted that.  and tux also optimises the rfc822
> Date header by updating a cached copy once per second 

I was thinking doing something like that.  One of the alternate designs
that I considered would have had a thread running in the background would
update the time once a second, but I ended up picking the cache-on-demand
solution because it's more portable to threadless MPMs.  (I also 
contemplated
having the accept loop in the worker MPM wake up once a second to update the
cached time, but that was just too ugly. :-)

> also -- you could eliminate the seconds = t / APR_USECS_PER_SEC
> calculation (death to all divisions!  especially the 64-bit ones)...
> because this division already happens inside the explode function:
>
> when you miss the cache and do a full explode, then subtract the 
> resulting
> tm_usec off the input time and you've got an apr_time_t which lands
> exactly on a second boundary.  use that as your cache key, and you can
> then do ranged comparisons (t >= cache->t && t < cache->t +
> APR_USECS_PER_SEC). 

The one problem I can see with this is that removing the division means
that you have to scan through the table to find out whether you have a
cache hit or not--which probably negates the performance benefit of
removing the division.

--Brian




Re: [PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by dean gaudet <de...@arctic.org>.
why is there a need for 15 entries?  if it's a multiprocess server then
there's only a need for 1 or 2 entries.  if it's a multithreaded server
then you need to lock the cache (which you're not doing :)

isn't the real win in eliminating both the divisions required by the
explode time functions and the divisions required by the printf?  fwiw,
tux does binary logging, and has an external tool to convert it to CLF...
and the spec committee accepted that.  and tux also optimises the rfc822
Date header by updating a cached copy once per second

also -- you could eliminate the seconds = t / APR_USECS_PER_SEC
calculation (death to all divisions!  especially the 64-bit ones)...
because this division already happens inside the explode function:

when you miss the cache and do a full explode, then subtract the resulting
tm_usec off the input time and you've got an apr_time_t which lands
exactly on a second boundary.  use that as your cache key, and you can
then do ranged comparisons (t >= cache->t && t < cache->t +
APR_USECS_PER_SEC).

-dean


[PATCH 2] Re: [PATCH] performance patch for mod_log_config

Posted by Brian Pane <bp...@pacbell.net>.
Cliff Woolley wrote:

>On Sun, 9 Sep 2001, Brian Pane wrote:
>
>>I think putting it in APR would work.  The one limitation I can think of
>>is that adding the cache in apr_explode_localtime() itself wouldn't be a
>>win because we'd have to add the overhead of a gettimeofday() call to
>>check whether the supplied time was indeed current (and thus susceptible
>>to caching).
>>
>
>It took me a minute to figure out why this was a problem, but you're
>right.  I don't really like the idea of combining time_now and
>explode_localtime, though.  What about some kind of deal like
>apr_explode_recent_localtime() which is the same as
>apr_explode_localtime() except that the input must never be older than
>TIME_CACHE_SIZE seconds?  Or is that too httpd-tailored?  If so, no sweat,
>make it an ap_ function instead of an apr_ function.
>
I think it's relatively httpd-specific, so I used 
ap_explode_recent_localtime().
Here's the revised patch, plus the new files include/util_time.h and 
server/util_time.c

--Brian

Index: modules/loggers/mod_log_config.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/loggers/mod_log_config.c,v
retrieving revision 1.68
diff -u -r1.68 mod_log_config.c
--- modules/loggers/mod_log_config.c    2001/08/27 20:50:01    1.68
+++ modules/loggers/mod_log_config.c    2001/09/10 06:42:02
@@ -463,9 +463,9 @@
     a problem with this, you can set the define.  -djg
     */
 #ifdef I_INSIST_ON_EXTRA_CYCLES_FOR_CLF_COMPLIANCE
-    apr_explode_localtime(&xt, apr_time_now());
+    ap_explode_recent_localtime(&xt, apr_time_now());
 #else
-    apr_explode_localtime(&xt, r->request_time);
+    ap_explode_recent_localtime(&xt, r->request_time);
 #endif
     if (a && *a) {              /* Custom format */
         apr_strftime(tstr, &retcode, MAX_STRING_LEN, a, &xt);





Re: [PATCH] performance patch for mod_log_config

Posted by Cliff Woolley <jw...@virginia.edu>.
On Sun, 9 Sep 2001, Brian Pane wrote:

> I think putting it in APR would work.  The one limitation I can think of
> is that adding the cache in apr_explode_localtime() itself wouldn't be a
> win because we'd have to add the overhead of a gettimeofday() call to
> check whether the supplied time was indeed current (and thus susceptible
> to caching).

It took me a minute to figure out why this was a problem, but you're
right.  I don't really like the idea of combining time_now and
explode_localtime, though.  What about some kind of deal like
apr_explode_recent_localtime() which is the same as
apr_explode_localtime() except that the input must never be older than
TIME_CACHE_SIZE seconds?  Or is that too httpd-tailored?  If so, no sweat,
make it an ap_ function instead of an apr_ function.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA




Re: [PATCH] performance patch for mod_log_config

Posted by Brian Pane <bp...@pacbell.net>.
William A. Rowe, Jr. wrote:

>Is this the right place to be caching, or should this become a straightforward
>optimization to apr's time.c functions?  I'd think the advantages are many for
>keeping 15 current seconds in apr, and would pay off across the board.  Within
>apr, we can always recalculate just the ms as well, for fun.
>
I think putting it in APR would work.  The one limitation I can think of
is that adding the cache in apr_explode_localtime() itself wouldn't be a
win because we'd have to add the overhead of a gettimeofday() call to
check whether the supplied time was indeed current (and thus susceptible
to caching).

But we could do something like this:

  apr_status_t  apr_explode_current_localtime(apr_exploded_time_t *t);

and presumably its GMT counterpart,

  apr_status_t  apr_explode_current_gmt(apr_exploded_time_t *t);


If that interface looks useful, I'll resubmit the patch in a form that
adds these functions to APR and calls them from mod_log_config.

--Brian




Re: [PATCH] performance patch for mod_log_config

Posted by Cliff Woolley <cl...@yahoo.com>.
On Sun, 9 Sep 2001, William A. Rowe, Jr. wrote:

> Is this the right place to be caching, or should this become a
> straightforward optimization to apr's time.c functions?  I'd think the
> advantages are many for keeping 15 current seconds in apr, and would
> pay off across the board.  Within apr, we can always recalculate just
> the ms as well, for fun.

+1

On a side note, I was doing a grep through the server for
apr_explode_localtime() to see where all it was used and came across this
hideously expensive-looking if/elseif/elseif/.../elseif/else block in
mod_rewrite for variable substitution.  Surely there's a faster way to
accomplish that little bit of logic than forty or so strcasecmp's for each
variable (in the worst case)...

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Re: [PATCH] performance patch for mod_log_config

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Is this the right place to be caching, or should this become a straightforward
optimization to apr's time.c functions?  I'd think the advantages are many for
keeping 15 current seconds in apr, and would pay off across the board.  Within
apr, we can always recalculate just the ms as well, for fun.

Bill

----- Original Message ----- 
From: "Brian Pane" <bp...@pacbell.net>
To: <de...@httpd.apache.org>
Sent: Sunday, September 09, 2001 9:46 PM
Subject: [PATCH] performance patch for mod_log_config


> The call to apr_explode_localtime() in mod_log_config is one of the more
> expensive operations in the httpd.  This patch attempts to reduce the
> overhead by caching the result.
> 
> --Brian
> 
> 
> Index: modules/loggers/mod_log_config.c
> ===================================================================
> RCS file: /home/cvspublic/httpd-2.0/modules/loggers/mod_log_config.c,v
> retrieving revision 1.68
> diff -r1.68 mod_log_config.c
> 447a448,498
>  >
>  > /* Cache of expanded timestamps:
>  >  *  Because apr_explode_localtime is an expensive function call,
>  >  *  we cache the exploded time and re-use it for the rest of the
>  >  *  current second.
>  >  */
>  >
>  > struct exploded_time_cache_element {
>  >     apr_int64_t t;
>  >     apr_exploded_time_t xt;
>  > };
>  >
>  > #define TIME_CACHE_SIZE 16
>  > static struct exploded_time_cache_element 
> exploded_time_cache[TIME_CACHE_SIZE];
>  >
>  > static void cached_explode_localtime(apr_exploded_time_t *xt, 
> apr_time_t t)
>  > {
>  >     apr_int64_t seconds = t / APR_USEC_PER_SEC;
>  >     struct exploded_time_cache_element *cache =
>  >         &(exploded_time_cache[seconds % TIME_CACHE_SIZE]);
>  >
>  >     /* The cache is implemented as a ring buffer.  Each second,
>  >      * it uses a different element in the buffer.  The timestamp
>  >      * in the element indicates whether the element contains the
>  >      * exploded time for the current second (vs the time
>  >      * 'now - TIME_CACHE_SIZE' seconds ago).  If the cached
>  >      * value is for the current time, we use it.  Otherwise,
>  >      * we compute the apr_exploded_time_t and store it in this
>  >      * cache element. Note that the timestamp in the cache
>  >      * element is updated only after the exploded time.  Thus
>  >      * if two threads hit this cache element simultaneously
>  >      * at the start of a new second, they'll both explode the
>  >      * time and store it.  I.e., the writers will collide, but
>  >      * they'll be writing the same value.
>  >      */
>  >     if (cache->t >= seconds) {
>  >         /* Note: If this memcpy ever takes more than TIME_CACHE_SIZE
>  >          * seconds, the value will be unpredictable (because this
>  >          * bucket in the ring buffer will have been recycled).
>  >          * This memcpy should never take multiple seconds, but to
>  >          * be safe we use a relatively large value for TIME_CACHE_SIZE.
>  >          */
>  >         memcpy(xt, &(cache->xt), sizeof(apr_exploded_time_t));
>  >     }
>  >     else {
>  >         apr_explode_localtime(xt, t);
>  >         memcpy(&(cache->xt), xt, sizeof(apr_exploded_time_t));
>  >         cache->t = seconds;
>  >     }
>  > }
>  >
> 466c517
> <     apr_explode_localtime(&xt, apr_time_now());
> ---
>  >     cached_explode_localtime(&xt, apr_time_now());
> 468c519
> <     apr_explode_localtime(&xt, r->request_time);
> ---
>  >     cached_explode_localtime(&xt, r->request_time);
> 
> 
> 
> 
> 
> 
> 
> 


Re: [PATCH] performance patch for mod_log_config

Posted by Cliff Woolley <cl...@yahoo.com>.
On Sun, 9 Sep 2001, Brian Pane wrote:

> The call to apr_explode_localtime() in mod_log_config is one of the more
> expensive operations in the httpd.  This patch attempts to reduce the
> overhead by caching the result.

Looks quite reasonable... I wouldn't mind seeing this as an ap_ function,
since other modules also use apr_explode_locatime and could take advantage
of it as well.  (Haven't we talked about doing this before?  What ever
happened to that?)

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA