You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "Roy T. Fielding" <fi...@apache.org> on 2002/06/11 00:04:19 UTC
apr_time_t --> apr_time_usec_t
I am tired of seeing this stupid change to the semantics of time_t
under Unix continue to cause bugs in every project that uses APR.
apr_time_t must be in seconds. If folks want APR to keep time in
microseconds, then they had bloody well change the type name
accordingly.
I know of one existing bug in httpd that I would consider a
showstopper, if I were RM, due to the way APR handles time.
In order to fix it, I am going to need to reinstate handling
of time in seconds, even if that means abandoning APR's routines.
....Roy
Re: apr_time_t --> apr_time_usec_t
Posted by Aaron Bannert <aa...@clove.org>.
On Mon, Jun 10, 2002 at 04:14:14PM -0700, Aaron Bannert wrote:
> Unfortunately, that type still has units of milliseconds.
>
> Seems like Roy needs an "apr_seconds_t", and apr_short_interval_time_t
> should be renamed "apr_milliseconds_t".
er, s/milliseconds/microseconds/
-aaron
Re: apr_time_t --> apr_time_usec_t
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 10 Jun 2002, Aaron Bannert wrote:
> On Mon, Jun 10, 2002 at 03:57:29PM -0700, Justin Erenkrantz wrote:
> > How about using the value we already have:
> >
> > typedef apr_int32_t apr_short_interval_time_t;
>
> Unfortunately, that type still has units of milliseconds.
> Seems like Roy needs an "apr_seconds_t", and apr_short_interval_time_t
> should be renamed "apr_milliseconds_t".
No, MICROseconds. For both of them.
apr_time_t: a 64 bit quantity specifying microseconds since the epoch
apr_interval_time_t: a 64 bit quantity specifying the microsecond
difference between two apr_time_t's.
apr_short_interval_time_t: a 32 bit quantity specifying the microsecond
difference between two apr_time_t's that are
very close together (what is it, like 36
minutes max?)
If you want some of those to be seconds or some alternative ones to be
seconds, fine, but let's be precise. :)
Re: apr_time_t --> apr_time_usec_t
Posted by Aaron Bannert <aa...@clove.org>.
On Mon, Jun 10, 2002 at 03:57:29PM -0700, Justin Erenkrantz wrote:
> How about using the value we already have:
>
> typedef apr_int32_t apr_short_interval_time_t;
Unfortunately, that type still has units of milliseconds.
Seems like Roy needs an "apr_seconds_t", and apr_short_interval_time_t
should be renamed "apr_milliseconds_t".
-aaron
Re: apr_time_t --> apr_time_usec_t
Posted by Brian Pane <br...@cnet.com>.
Justin Erenkrantz wrote:
>>Ryan Bloom wrote:
>>
>>This is going to break EVERY apr app out there. I have no problem with
>>the change, but everything is going to be broken. Is there any way to
>>do this by creating an apr_time32_t, or something that will keep the
>>default at 64-bit, and allow people to use 32 if they specify it?
>>
>>
>
>How about using the value we already have:
>
>typedef apr_int32_t apr_short_interval_time_t;
>
>And, since there *are* processors that are 64-bit out there, I'm
>not entirely sure if it makes sense for us to go back to 32-bit
>for performance reasons. -- justin
>
>
I think it's better to maintain the microseconds part, but to split
the seconds and microseconds into separate fields in a timeval structure.
Sometimes we actually want microseconds. They're useful, for example,
when generating unique IDs or when doing custom performance monitoring.
Most of the time, in the httpd at least, we just want seconds. We get
both for free from gettimeofday(), so why not keep both?
Also, apr_short_interval_time_t isn't really a good choice for holding
a time_t. That signed 32-bit int is going to overflow in 2038.
--Brian
Re: apr_time_t --> apr_time_usec_t
Posted by Justin Erenkrantz <je...@apache.org>.
> Ryan Bloom wrote:
>
>This is going to break EVERY apr app out there. I have no problem with
>the change, but everything is going to be broken. Is there any way to
>do this by creating an apr_time32_t, or something that will keep the
>default at 64-bit, and allow people to use 32 if they specify it?
How about using the value we already have:
typedef apr_int32_t apr_short_interval_time_t;
And, since there *are* processors that are 64-bit out there, I'm
not entirely sure if it makes sense for us to go back to 32-bit
for performance reasons. -- justin
Re: apr_time_t --> apr_time_usec_t
Posted by Brian Pane <br...@cnet.com>.
Ryan Bloom wrote:
>This is going to break EVERY apr app out there. I have no problem with
>the change, but everything is going to be broken. Is there any way to
>do this by creating an apr_time32_t, or something that will keep the
>default at 64-bit, and allow people to use 32 if they specify it?
>
How about leaving apr_time_t unchanged and adding an apr_timeval:
typedef struct {
unsigned tv_sec;
unsigned tv_usec;
} apr_timeval;
plus
apr_status_t apr_timeval_now(apr_timeval* time)
to complement (not replace) the existing apr_time_now()
and apr_timeval versions of other time functions.
--Brian
RE: apr_time_t --> apr_time_usec_t
Posted by Ryan Bloom <rb...@covalent.net>.
This is going to break EVERY apr app out there. I have no problem with
the change, but everything is going to be broken. Is there any way to
do this by creating an apr_time32_t, or something that will keep the
default at 64-bit, and allow people to use 32 if they specify it?
I did a quick search, Dean didn't mention why he made the change to
64-bit back when he committed the code in 2000, and there doesn't seem
to have been a conversation about it at the time.
Ryan
----------------------------------------------
Ryan Bloom rbb@covalent.net
645 Howard St. rbb@apache.org
San Francisco, CA
> -----Original Message-----
> From: Brian Pane [mailto:brian.pane@cnet.com]
> Sent: Monday, June 10, 2002 3:39 PM
> To: dev@apr.apache.org
> Subject: Re: apr_time_t --> apr_time_usec_t
>
> Roy T. Fielding wrote:
>
> > I am tired of seeing this stupid change to the semantics of time_t
> > under Unix continue to cause bugs in every project that uses APR.
> > apr_time_t must be in seconds. If folks want APR to keep time in
> > microseconds, then they had bloody well change the type name
> > accordingly.
>
>
> +1. I'm tired of taking the perfomance hit of
> 64-bit divisions on 32-bit CPUs to retrieve the
> time_t.
>
> In order to fix it, though, we'd really need to change
> the implementation in some way that breaks old code at
> compile time. Otherwise, if apr_time_t remains a 64-bit
> int but its meaning changes by a factor of a million, lots
> of bugs in people's APR-based apps aren't going to show up
> until runtime.
>
>
> > I know of one existing bug in httpd that I would consider a
> > showstopper, if I were RM, due to the way APR handles time.
> > In order to fix it, I am going to need to reinstate handling
> > of time in seconds, even if that means abandoning APR's routines.
>
>
> Don't keep us in suspense...what's the bug?! :-)
>
> --Brian
>
Re: apr_time_t --> apr_time_usec_t
Posted by Brian Pane <br...@cnet.com>.
Roy T. Fielding wrote:
> I am tired of seeing this stupid change to the semantics of time_t
> under Unix continue to cause bugs in every project that uses APR.
> apr_time_t must be in seconds. If folks want APR to keep time in
> microseconds, then they had bloody well change the type name
> accordingly.
+1. I'm tired of taking the perfomance hit of
64-bit divisions on 32-bit CPUs to retrieve the
time_t.
In order to fix it, though, we'd really need to change
the implementation in some way that breaks old code at
compile time. Otherwise, if apr_time_t remains a 64-bit
int but its meaning changes by a factor of a million, lots
of bugs in people's APR-based apps aren't going to show up
until runtime.
> I know of one existing bug in httpd that I would consider a
> showstopper, if I were RM, due to the way APR handles time.
> In order to fix it, I am going to need to reinstate handling
> of time in seconds, even if that means abandoning APR's routines.
Don't keep us in suspense...what's the bug?! :-)
--Brian
Re: apr_time_t --> apr_time_usec_t
Posted by "Roy T. Fielding" <fi...@apache.org>.
On Monday, June 10, 2002, at 04:11 PM, William A. Rowe, Jr. wrote:
> At 05:04 PM 6/10/2002, you wrote:
>> I am tired of seeing this stupid change to the semantics of time_t
>> under Unix continue to cause bugs in every project that uses APR.
>
> I must have missed that discussion traveling. Pointers please?
Just do a search for 1000000 or USEC in the cvs archives. I've seen at
least two dozen separate bug fixes applied to APR and httpd that were
directly caused by people changing time_t to apr_time_t and expecting
it to work. The latest one floated by this weekend. Bad interfaces
are evidenced by repetition of the same bugs over time.
>> apr_time_t must be in seconds. If folks want APR to keep time in
>> microseconds, then they had bloody well change the type name
>> accordingly.
>
> apr_time_t must nothing :-) Let's discuss *should(s)*
No. I will veto the change from time_t to apr_time_t due to misleading
changes in semantics. I am going back to the state wherein httpd was
robust. Since my change will both reduce bugs and improve performance,
without sacrificing any known features, there is sufficient justification
for making it in httpd.
You can debate how this should effect APR all you like. I'd prefer
a solution that is general to APR. I don't know of any APR client apps
that make use of microseconds other than flood and ab.
> time_t is seconds. I love the idea of apr_time_usec_t and apr_time_sec_t
> names rather that something as ambigous as apr_time_t (which is
> misleading,
> I agree.)
Fine, that is one solution.
> As far as adopting apr_time_sec_t throughout, you may be looking forward
> to your retirement party before a signed 32 bit apr_time_sec_t blows
> chunks.
> Having coded against Y2K since 1989, I'd absolutely veto this suggestion
> for general adoption. Specific cases, fine.
time_t is not limited to 32 bits. I never mentioned 32 bits. You must
be referring to someone else. Doing 64 bit divides and multiplies are
bad for performance, but that is done because we are storing microseconds
even though we never use microseconds. Eliminate the microseconds, or
move it to a separate field, but keep the 64 bits. I personally think
it a waste of time (no pun intended) to worry about a 32bit time_t
when time_t is only 32 bits on machines that only have a 32bit interface
to their time operations, and hence our clock will still roll-over
regardless of how many extra bits we use to store our copy of time_t.
I will discuss httpd changes on dev@httpd -- this thread was just to
get a little frustration off my chest regarding APR design decisions
that are made based on the theory that they "might some day be needed"
and then never changed (in spite of several requests on my part) based
on the theory that they might break some mythical "user" of APR. It's
time to get the clue stick out of the closet. From now on, apr_time_t
is considered harmful.
....Roy
Re: apr_time_t --> apr_time_usec_t
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 05:04 PM 6/10/2002, you wrote:
>I am tired of seeing this stupid change to the semantics of time_t
>under Unix continue to cause bugs in every project that uses APR.
I must have missed that discussion traveling. Pointers please?
>apr_time_t must be in seconds. If folks want APR to keep time in
>microseconds, then they had bloody well change the type name
>accordingly.
apr_time_t must nothing :-) Let's discuss *should(s)*
time_t is seconds. I love the idea of apr_time_usec_t and apr_time_sec_t
names rather that something as ambigous as apr_time_t (which is misleading,
I agree.)
As far as adopting apr_time_sec_t throughout, you may be looking forward
to your retirement party before a signed 32 bit apr_time_sec_t blows chunks.
Having coded against Y2K since 1989, I'd absolutely veto this suggestion
for general adoption. Specific cases, fine.
>I know of one existing bug in httpd that I would consider a
>showstopper, if I were RM, due to the way APR handles time.
>In order to fix it, I am going to need to reinstate handling
>of time in seconds, even if that means abandoning APR's routines.
Please share, I'm certain a few more pairs of eyes could prove useful.
++1 on distinguishing apr_time_t to be more meaningful, though!
Re: apr_time_t --> apr_time_usec_t
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 10 Jun 2002, Cliff Woolley wrote:
> No, I committed a patch for this on May 8. It's still broken for you? In
> HEAD? On Unix or Win32?
PS: See http://nagoya.apache.org/bugzilla/show_bug.cgi?id=8760 .
--Cliff
Re: apr_time_t --> apr_time_usec_t
Posted by "Roy T. Fielding" <fi...@apache.org>.
>>> If-Modified-Since doesn't work because an HTTP time based on
>>> seconds x100000 is being compared to a file modification time
>>> based directly on microseconds.
>>
>> I thought I fixed that already!? Oh boy, did the patch not get
>> committed?
>> It might be sitting in the PR waiting for somebody to test it.
>>
>> I'll go check.
>
> No, I committed a patch for this on May 8. It's still broken for you? In
> HEAD? On Unix or Win32?
No, I missed that you had mostly fixed it --- I had saved the original
report for later work.
I still think it is insane to multiply or divide every time we want to
use seconds. Not a showstopper, though.
....Roy
Re: apr_time_t --> apr_time_usec_t
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 10 Jun 2002, Cliff Woolley wrote:
> On Mon, 10 Jun 2002, Roy T. Fielding wrote:
>
> > If-Modified-Since doesn't work because an HTTP time based on
> > seconds x100000 is being compared to a file modification time
> > based directly on microseconds.
>
> I thought I fixed that already!? Oh boy, did the patch not get committed?
> It might be sitting in the PR waiting for somebody to test it.
>
> I'll go check.
No, I committed a patch for this on May 8. It's still broken for you? In
HEAD? On Unix or Win32?
--Cliff
Re: apr_time_t --> apr_time_usec_t
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 10 Jun 2002, Roy T. Fielding wrote:
> If-Modified-Since doesn't work because an HTTP time based on
> seconds x100000 is being compared to a file modification time
> based directly on microseconds.
I thought I fixed that already!? Oh boy, did the patch not get committed?
It might be sitting in the PR waiting for somebody to test it.
I'll go check.
--Cliff
Re: apr_time_t --> apr_time_usec_t
Posted by "Roy T. Fielding" <fi...@apache.org>.
On Monday, June 10, 2002, at 03:22 PM, Cliff Woolley wrote:
> On Mon, 10 Jun 2002, Roy T. Fielding wrote:
>
>> I know of one existing bug in httpd that I would consider a
>> showstopper, if I were RM, due to the way APR handles time.
>
> Are you going to tell me what it is? :)
If-Modified-Since doesn't work because an HTTP time based on
seconds x100000 is being compared to a file modification time
based directly on microseconds.
....Roy
Re: apr_time_t --> apr_time_usec_t
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 10 Jun 2002, Roy T. Fielding wrote:
> I know of one existing bug in httpd that I would consider a
> showstopper, if I were RM, due to the way APR handles time.
Are you going to tell me what it is? :)