> APR needs to be written and made correct once. there will be more folks
> using APR than there are needs to fix the time code within APR.
I'd certainly hope so, but that is not a very relevant metric.
There will be just as many folks, if not more, using it after it
is fixed.
> time arithmetic is way easier on a single scalar than it is on multiple
> scalars. contrast:
>
> diff = now - then;
>
> vs.
>
> diff.sec = now.sec - then.sec;
> long tmp = (long)now.usec - (long)then.usec;
> if (tmp < 0) {
> tmp += 1000000;
> --diff.sec;
> }
> diff.usec = tmp;
No, contrast
diff = apr_time_interval(then, now);
apr_time_t is a data type. The above can be a macro if you want it fast,
but the point is that every single client of APR should not be dependent
on knowing the exact representation of time and remembering that they
have to divide it by 100000 just to get something useful.
> > If you want to really optimize it for speed, include a third
> > variable for the native OS time_t, and a const char * for storing the
> > rfc822 date string the first time it is converted.
>
> my change to a single scalar was not an optimisation, it was intended
> to make it more likely that users of APR would generate correct time
> arithmetic. i'm frequently willing to throw away small amounts
> of performance if it's more likely folks will write correct code.
I have seen well over 30 commits fixing bugs specifically due to this
notion that you can subtract one unknown scalar from another unknown
scalar and still remain within the bounds of some other unknown scalar.
And another 20 or so fixing places where someone forgot to divide by
or multiply by 100000. The theory was wrong.
> the performance bug is not the APR time representation -- it's the httpd
> code which calls ap_now() for every single request and then does all
> the module 10 arithmetic to format it into a Date: and log timestamp.
>
> the solution that all the performance winning webservers use is a
> separate thread which wakes up once a second to generate new base-10
> representations of the time.
>
> now that 2.0 has all the zero-copy machinery i think you've got what
> you need to share the timestamp strings amongst multiple threads without
> worrying about the safety issues. (bill's suggested two buffer method
> works fine unless you for some reason have a task sleep for 2 seconds...
> such as when a system is under a hellish load.)
No question about that, but my statement was serious: this implementation
has demonstrated to be both more error-prone and slower than storing
seconds as seconds. I would fix it within APR because that makes better
code available to all of the clients. Even if we do fix it within httpd,
my solution would be to stop using the APR time functions.
> > Regardless, APR and httpd have assumptions strewn throughout the code
> > that apr_time_t is a long long in microseconds, which isn't native
> > on any platform and makes every time operation slower for no reason.
>
> either you spread code around which assumes that you've got two fields,
> one seconds and one microseconds, or you assume that you've got one field
> in microseconds. i don't see how either assumption is any worse than the
> other.
An abstract data type calls for accessor functions/macros. But I don't
want to argue about this. If I get the time (this just hasn't been a
priority for me compared to the other apr/httpd issues), I'll code up
a replacement and it will beat the current implementation in terms of
both speed and understandability. If it doesn't, I won't commit it.
....Roy
