Re: [PATCH] small optimization in ap_meets_conditions() and questions

[Jean-Jacques Clar <JJ...@novell.com>]

Since the target value is in second, it should be possible to use
r->request_time instead of calling apr_time_now().
I ran a load test on my box and did not ever see a difference between
the sec value in r->request_time and the one returned by
apr_time_now().
 
end of the patch part......
 
-----------------------------------------------
 
Questions: 
1- 
It should also be valuable to change the return format of
apr_date_parse_http() to be in seconds instead of uSEC.
This is a snippet of the last part of that function:
 
--- boc ---
    /* ap_mplode_time uses tm_usec and tm_gmtoff fields, but they
haven't 
     * been set yet. 
     * It should be safe to just zero out these values.
     * tm_usec is the number of microseconds into the second.  HTTP
only
     * cares about second granularity.
     * tm_gmtoff is the number of seconds off of GMT the time is.  By
     * definition all times going through this function are in GMT, so
this
     * is zero. 
     */
    ds.tm_usec = 0;
--- eoc ---
 
and the time is exploded to match the apr_time_t format.
 
A call to apr_date_parse_httpd() is usually followed by a call to
apr_time_sec(), which do a 64 bits division to get the time in second.
What about having a new function called apr_date_parse_httpd_sec(),
which skip the end part of apr_time_exp_get() and return seconds?
It implies a small change to apr_time_exp_get() and moving the core
code in apr_time_exp_get() to apr_time_exp_get_sec().:
 
--- boc ---
APR_DECLARE(apr_status_t) apr_time_exp_get(apr_time_t *t,
apr_time_exp_t *xt)
{
    apr_status_t ret;
 
    ret = apr_time_exp_get_sec(t, xt);
    if (ret == APR_SUCCESS)
        *t = *t * APR_USEC_PER_SEC + xt->tm_usec;
 
    return ret;
}
--- eoc ---
 
-----------
 
2: 
What about creating a new define called: 
#define APR_HAS_USEC.
 
The else of that option could allow apache to run using ONLY sec for
all apr_time_t field removing all 64 bits division to transform uSEC to
sec.
The size of the apr_time_t variables could also be changed to 32 bits
and calls to gettimeofday() changed to time().
Why is apache using uSEC, when time variables are mostly used in
seconds at run-time, and that http requirements use seconds from my
modest knownledge.
The performance gain is minor but measurable: between 1 and 2.5% on my
box (from 1 to 4 CPUs).
 
-----------
 
3:
On having a time variable in the mpm keeping updated sec and uSEC
field:
NetWare has its main thread resuming every 1000000 uSEC (1 sec) to do
the idle server maintenance.
I am thinking of using a set of functions like the following ones:
 
--- boc ---
static apr_int32_t mpm_clock_initialized;
static apr_time_t now;
struct timeval tv;
 
static void synchonize_usec_clock( void )
{
  static apr_int32_t counter;
 
  if (mpm_clock_initialized) {
    if(++counter > 4) {
      gettimeofday(&tv, NULL);
      now = tv.tv_sec * APR_USEC_PER_SEC + tv.tv_usec;
      counter = 0;
    }
    else
      now += SCOREBOARD_MAINTENANCE_INTERVAL;
  }
  else {
    gettimeofday(&tv, NULL);
    now = tv.tv_sec * APR_USEC_PER_SEC + tv.tv_usec;
    mpm_clock_initialized = 1;
  }
}
 
AP_DECLARE(apr_time_t) ap_time_now(void)
{
  if (mpm_clock_initialized) 
    return now;
  else
    return apr_time_now();
}
--- eoc ---
 
The synchonize_usec_clock() function is called every second by the main
thread. 
It synchronizes the time fields every 5 seconds using the
gettimeofday() function. 
Calls to apr_time_now() in apache could then be replaced with call to
ap_time_now().
 
-----------
 
Thank you,
 
Jean-Jacques