Re: [SUMMARY] time discussion
A fine summary of the situation. On Friday, July 12, 2002, at 12:42 PM, William A. Rowe, Jr. wrote: I. We represent all time quantum in the same scale throughout APR. That scale is in microseconds. Which is goodness, because we don't ever have to go back to docs and ask, Does that function take seconds or apr time? It's easy to keep track of that within APR. What is hard is dealing with both APR and other libraries within the code of APR users, since the rest of the universe thinks in seconds. The accessor functions were a significant improvement. II. Performance is an issue, we are attempting to reclaim CPU cycles lost converting, especially between seconds and microseconds, both internally and externally (by other apps.) And everyone agrees we want this as fast as possible, without introducing bugs due to [whatever sort of] programmer confusion. Yes, +1 on applying the binary microseconds patch -- its far better than the current state. III. The existing name is an issue to Roy and others who are confused by the similarity between apr_time_t and time_t (in the ANSI/POSIX definitions.) And I agree it's an obstacle to 1) porting old code to APR, and 2) folks quickly getting comfortable with APR, when they are just learning the library. I haven't been confused by it. I just have a better memory of the number of times that you have patched various aspects of httpd and apr-util due to people confusing them, or simply changing the interface using mass find/replace. Most of those bugs were introduced by the authors of APR, so I fail to see how just learning applies. IV. Without sacrificing resolution, I put forward a proposal that we use a binary representation of microseconds. Mr. Stoddard has determined that the binary representation we presented does reduce the overall cpu instructions and clock cycles in httpd request processing, as expected. This has two benefits. Scalar math operations simply work; computation of deltas doesn't require additional carry operations. However, seconds can be quickly grabbed with a binary shift, so there is no huge integer division to contend with. It's an all-around performance win. However, it's mildly confusing to work with, without the macros. Those macros need to be thoroughly vetted for range and overflow errors, etc. The casts should be removed and the interval time really should have the same size as epoch time. V. Aaron and others submit that we should change the name of the type if we change the scale, to assure our APR library users aren't tripped up by casual msec = t / 1000 computations in their existing code. This just happens to coincide with Roy's concerns in (III.) above. And with (III.) above, it just makes good sense to pick new names for this new type, IF we are going to have a contract with the programmers about the representation. We can have compatibility macros until the old symbols are deprecated, and Aaron and others who are concerned with catching all instances of the old usage can disable the compat macros. Er, that would have been a good idea, had it been deployed earlier. VI. Brian and others have asked that we have an undefined scalar value [with no contract to the users about it's representation.] Roy and others object, due to overflow and range considerations, and binary compatibility considerations [as it's all in macros that aren't updated by new binaries]. I really don't see a win here. Why have no contract? We aren't hiding the definitions within accessor functions behind an opaque type. There is NO type safety when you use C scalars for a type. And the code can never be binary drop-in replaceable, the time manipulation was all compiled into the user's code from the macros, it isn't buried safely inside of APR. I just don't believe in partially-implemented ADTs. It needs to be either abstract to hide implementation details or written in stone such that the implementation details are guaranteed across platforms. One or the other is okay with me, but not both. VII. Ryan and others submit that we need two types, in fact; one absolute measure (from epoch 1.1.1970) and one 'interval' or 'delta' that represents a span, rather than a time. This is the case in APR today. And we all agree here. The key words, time and span make the most sense after I considered it. span is fairly well adopted in the C/C++/STL world. VIII. From all of the above came the original discussion of naming. Ryan and others believe we should not change the name of the type, whatsoever. The sub-argument is between a strongly defined name contract, e.g. busec in the identifier, or a completely ambigious name with no contract of the scale's unit. And I stand on a strongly named, intuitive names that warn you not to just pass around seconds. That leaves us with something like; apr_time_busec_t apr_span_busec_t which conveys that the span is measured in
[SUMMARY] time discussion
At 01:47 PM 7/12/2002, David Reid wrote Can someone simply restate what issue needs fixing. No more hand waving or IRC chats, a simple email explaining the issue and what needs fixed. I will try to do so in a fair and balanced way; I. We represent all time quantum in the same scale throughout APR. That scale is in microseconds. II. Performance is an issue, we are attempting to reclaim CPU cycles lost converting, especially between seconds and microseconds, both internally and externally (by other apps.) III. The existing name is an issue to Roy and others who are confused by the similarity between apr_time_t and time_t (in the ANSI/POSIX definitions.) IV. Without sacrificing resolution, I put forward a proposal that we use a binary representation of microseconds. Mr. Stoddard has determined that the binary representation we presented does reduce the overall cpu instructions and clock cycles in httpd request processing, as expected. V. Aaron and others submit that we should change the name of the type if we change the scale, to assure our APR library users aren't tripped up by casual msec = t / 1000 computations in their existing code. This just happens to coincide with Roy's concerns in (III.) above. VI. Brian and others have asked that we have an undefined scalar value [with no contract to the users about it's representation.] Roy and others object, due to overflow and range considerations, and binary compatibility considerations [as it's all in macros that aren't updated by new binaries]. VII. Ryan and others submit that we need two types, in fact; one absolute measure (from epoch 1.1.1970) and one 'interval' or 'delta' that represents a span, rather than a time. This is the case in APR today. VIII. From all of the above came the original discussion of naming. Ryan and others believe we should not change the name of the type, whatsoever. The sub-argument is between a strongly defined name contract, e.g. busec in the identifier, or a completely ambigious name with no contract of the scale's unit. IX. Roy's original comments yesterday went back to item (II.) above, and reintroduced to the optimization discussion the options of either using seconds to those apr functions that don't need precision, and/or replacing our time definition with a structure (seperate seconds and useconds fields.) These options were debated/voted upon several times before on the APR list. I hope this is a balanced and fair summary of the discussion to date. My unbalanced opinions are a topic for another post. Bill
Re: [SUMMARY] time discussion
William A. Rowe, Jr. wrote: I hope this is a balanced and fair summary of the discussion to date. Lovely. -- === Jim Jagielski [|] [EMAIL PROTECTED] [|] http://www.jaguNET.com/ A society that will trade a little liberty for a little order will lose both and deserve neither - T.Jefferson
Re: [SUMMARY] time discussion
Now my own comments; I. We represent all time quantum in the same scale throughout APR. That scale is in microseconds. Which is goodness, because we don't ever have to go back to docs and ask, Does that function take seconds or apr time? II. Performance is an issue, we are attempting to reclaim CPU cycles lost converting, especially between seconds and microseconds, both internally and externally (by other apps.) And everyone agrees we want this as fast as possible, without introducing bugs due to [whatever sort of] programmer confusion. III. The existing name is an issue to Roy and others who are confused by the similarity between apr_time_t and time_t (in the ANSI/POSIX definitions.) And I agree it's an obstacle to 1) porting old code to APR, and 2) folks quickly getting comfortable with APR, when they are just learning the library. IV. Without sacrificing resolution, I put forward a proposal that we use a binary representation of microseconds. Mr. Stoddard has determined that the binary representation we presented does reduce the overall cpu instructions and clock cycles in httpd request processing, as expected. This has two benefits. Scalar math operations simply work; computation of deltas doesn't require additional carry operations. However, seconds can be quickly grabbed with a binary shift, so there is no huge integer division to contend with. It's an all-around performance win. However, it's mildly confusing to work with, without the macros. Those macros need to be thoroughly vetted for range and overflow errors, etc. V. Aaron and others submit that we should change the name of the type if we change the scale, to assure our APR library users aren't tripped up by casual msec = t / 1000 computations in their existing code. This just happens to coincide with Roy's concerns in (III.) above. And with (III.) above, it just makes good sense to pick new names for this new type, IF we are going to have a contract with the programmers about the representation. We can have compatibility macros until the old symbols are deprecated, and Aaron and others who are concerned with catching all instances of the old usage can disable the compat macros. VI. Brian and others have asked that we have an undefined scalar value [with no contract to the users about it's representation.] Roy and others object, due to overflow and range considerations, and binary compatibility considerations [as it's all in macros that aren't updated by new binaries]. I really don't see a win here. Why have no contract? We aren't hiding the definitions within accessor functions behind an opaque type. There is NO type safety when you use C scalars for a type. And the code can never be binary drop-in replaceable, the time manipulation was all compiled into the user's code from the macros, it isn't buried safely inside of APR. VII. Ryan and others submit that we need two types, in fact; one absolute measure (from epoch 1.1.1970) and one 'interval' or 'delta' that represents a span, rather than a time. This is the case in APR today. And we all agree here. The key words, time and span make the most sense after I considered it. span is fairly well adopted in the C/C++/STL world. VIII. From all of the above came the original discussion of naming. Ryan and others believe we should not change the name of the type, whatsoever. The sub-argument is between a strongly defined name contract, e.g. busec in the identifier, or a completely ambigious name with no contract of the scale's unit. And I stand on a strongly named, intuitive names that warn you not to just pass around seconds. That leaves us with something like; apr_time_busec_t apr_span_busec_t which conveys that the span is measured in buseconds. I'm suggesting the time/span before busec so we don't have to go about renaming EVERY SINGLE apr_time_fn() to a new type. Can we all live with apr_time_() functions addressing apr_time_busec_t values? Or do we have to go to the extreme of renaming these all apr_time_busec_fn()? IX. Roy's original comments yesterday went back to item (II.) above, and reintroduced to the optimization discussion the options of either using seconds to those apr functions that don't need precision, and/or replacing our time definition with a structure (seperate seconds and useconds fields.) These options were debated/voted upon several times before on the APR list. To the idea of both a seconds and a fine-resolution time type in APR... I say no friggin way. All of us have introduced bugs in code at one time or another by mixing up our seconds, mseconds and useconds, no? One scale for time in APR is sufficient. That's the way NSPR went as well, IIRC. To the other idea of going back to a structure, that has a huge performance penalty when you need to compute deltas. Simple add/subtract becomes 10+ cpu instructions. That's the main objection
Re: [SUMMARY] time discussion
On Fri, Jul 12, 2002 at 02:42:22PM -0500, William A. Rowe, Jr. wrote:
And with (III.) above, it just makes good sense to pick new names for this
new type, IF we are going to have a contract with the programmers about
the representation. We can have compatibility macros until the old symbols
are deprecated, and Aaron and others who are concerned with catching all
instances of the old usage can disable the compat macros.
Nope, compatibility macros can't work since the claim is that people
are directly altering the values. If they used the functions, then
the complaint about backwards-compatibility would be moot.
VI. Brian and others have asked that we have an undefined scalar value
[with no contract to the users about it's representation.] Roy and
others
object, due to overflow and range considerations, and binary
compatibility
considerations [as it's all in macros that aren't updated by new
binaries].
I really don't see a win here. Why have no contract? We aren't hiding the
definitions within accessor functions behind an opaque type. There is NO
type safety when you use C scalars for a type.
And the code can never be binary drop-in replaceable, the time manipulation
was all compiled into the user's code from the macros, it isn't buried safely
inside of APR.
I maintain that it should be undefined so that on source-compatible
releases, we can switch the types. On binary-compatible release, we
can't touch it though. (Greg and Ryan have defined this well.)
I'm slowly beginning to believe that apr_time_t *should* be entirely
opaque.
#define apr_time_diff(time1,time2) time1-time2
...
But, in APR 2.0 (or the next source-compatible release), if we wanted
to we could make it a function such that (not that we would do
this example for reasons you point out later):
apr_span_t apr_time_diff(apr_time_t t1, apr_time_t t2)
{
apr_span_t t3;
t3.sec = t1.sec - t2.usec;
t3.usec = t1.usec - t2.usec;
return t3;
}
The fact that we exposed the time structure at all is the reason that
we are in this predicament. Merely changing the type name would not
be a win, either. If we make it unspecified, we have to make sure
that direct modification of apr_time_t is not allowed.
VIII. From all of the above came the original discussion of naming. Ryan and
others believe we should not change the name of the type, whatsoever.
The sub-argument is between a strongly defined name contract, e.g.
busec in the identifier, or a completely ambigious name with no
contract
of the scale's unit.
And I stand on a strongly named, intuitive names that warn you not to just
pass around seconds. That leaves us with something like;
apr_time_busec_t
apr_span_busec_t
which conveys that the span is measured in buseconds.
I'm suggesting the time/span before busec so we don't have to go about
renaming EVERY SINGLE apr_time_fn() to a new type. Can we all live
with apr_time_() functions addressing apr_time_busec_t values? Or do
we have to go to the extreme of renaming these all apr_time_busec_fn()?
No, if you buy in the apr_time_busec_t argument, all of the functions
have to be renamed. The reason is that I can come along with a
apr_time_dsec_t implementation. If we have an explicit contract,
we MUST make the API specific to that contract. No apr_time_now(),
etc. It has to be apr_time_busec_now(), etc.
Personally, I think the proponents of the busec contract are going
to paint ourselves in a tight corner if we do find something wrong
with that implementation down the road. I would much prefer that
we allow ourselves to change implementations without changing the
API. I believe that the explicit contract solution for busec will
come back to haunt us. -- justin
