apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject RE: more notes on the apr_time_t issue
Date Mon, 15 Jul 2002 02:20:36 GMT
At 03:21 PM 7/14/2002, Ryan Bloom wrote:
>I can accept apr_time.
> >
> > They won't.  They will ask WTF is a busec?  They will look, and 
> discover that
> > it's a binary usec.  In the doc sections for both apr_time and 
> apr_timediff/span
> > we point them @see {group} apr_time_macros.  There they find all the
> > macros.
>
>That is unacceptable.  The types have to make sense, or they just aren't
>useful.  If everytime somebody new wants to use APR, they have to look
>up what apr_utime or apr_busec or apr_foo is, they will drop APR very
>quickly.

The bottom line is; we have only one representation of time.  For all apr_foo()
functions.  In some functions, it's epoch 1.1.70, in others it's a span.

>So my names are wrong, but the concept is the same.  More macros than
>you can keep straight to fill in the time structure.  The advantage to
>the current model, is that there is one macro:  APR_SEC_FROM_USEC.

No, that's not entirely fair, and no, we can have an APR_TIME_PER_SEC
value.  I'm not suggesting we hide this scalar behind a curtain.  The other
macros are for convenience, not to be a pain in the ass.  Others disagree,
but once we hit binary compatibility, these can never change.

The macros, however, are considerably faster on all platforms, they don't
rely on the compiler doing all the optimization.  That's why the macros are
infinitely preferable to the constant.  They are [will also be] vetted so that
the correct behavior is on the library, not the developer's math.

>In fact, you added one more macro, the nsec variant, so now you have
>_10_ macros to get information out of a simple time type.  That doesn't
>seem incredibly extreme to you?

Not in terms of performance.  We have a four real macros;

apr_time_sec_get(time) [which is the same as...]
apr_time_to_sec(time)
apr_time_from_sec(sec)

apr_time_xsec_get(time) [gets only the fraction of a second]
apr_time_to_xsec(time) [converts complete value by scale]
apr_time_from_xsec(xsec) [converts complete value by scale]
apr_time_xmake(sec, xsec);

So xsec can be an nsec, usec or msec, depending on the app.
So WHAT?  They all follow an incredibly simple pattern.

I have no problem fixing the ambiguity between _xsec_get and _to_xsec,
since the difference is subtle.  Perhaps apr_time_frac_to_xsec(time)
instead of _xsec_get?  (saying, only the fractional portion of a second?)

And I'm dropping the idea of apr_time_xsec_put.  The reason is simple,
if you pass zero as sec to an apr_time_xmake, all of the seconds math
drops out (being a constant zero) and it's simply as efficient.  It's also
clearer than _xsec_put, as that macro had overflow issues.

> > >Apr_busec_add
> > >Apr_busec_sub
>
>I may be wrong, but I explicitly remember seeing those macros being
>discussed.

You did.  The discussion was bogus since these are scalars, and we
won't adopt a non scalar type.  But apparently folks are still whining for
them, so I won't object (but I won't ever use them myself).

We cannot implement this as a non-scalar and get better performance.

> > apr_time_now ... the fn name WON'T CHANGE.
>
>So what, the function exists.  Yes, it exists today, but I was merely
>pointing out that you are going from 14 time functions and 1 macro to 14
>functions and 10 macros (was 8 in my last e-mail, but the [n] variant
>wasn't in my count).

There are four macros with three-four flavors each.  All that distinguishes
them is scale, so what possible confusion does that introduce?  If you
are twisting a number into or out of apr_time, you better understand the
scale of that non-apr value!!!

Win32 has internal _100ns flavors to directly deal with Win32 times
in an optimized way.  No reason for the user to ever see those macros,
but again, those macros will assure us we don't misstep.

> > >Apr_explode_time
> > >Apr_explode_localtime
> >
> > Please reassure me that we've fixed all those old names long ago,
> > and you were just recalling the old forms.
>
>Nope, those came straight out of the headers.

Yes, they are already in renames_pending, to become apr_time_() fns.

> > >22 functions/macros to deal with time. 8 of those are because we are
> > >storing time in a format that nobody else expects.
> >
> > We have those today.  We have them because we deal in uniform
> > apr_time_t RIGHT NOW TODAY.  This whole proposal changes nothing
> > in that department.
>
>Sure it does.  We have 1 macro today, now we need 10!

Quit it with the ten bs :)  Ok, four macros of three-four flavors, instead of
one macro const value.  We've been using those macros for several weeks,
or had you missed that?

There is no other impact in complexity here, IMHO.

>great, that means that EVERY single call that deals with time in EVERY
>single APR app needs to change.  And, it means that calling those
>functions will just become a painful exercise for new programmers.
>"Let's see, I need to pass an apr_time value, which is a binary usec
>implementation, now, how do I get binary usec's again?  I guess I'll go
>read the docs to find out."  Programming used to make sense, because the
>types made sense and for simple things, like timeouts, programmers could
>use simple values.  Now, our timeouts are incredibly complex, which
>makes using them painful.   :-(

No, this is _NO_ more difficult than going back to the docs to figure out
what APR_FOO_FLAG values you must use.  Look at any sources, and
you will see examples (including apr's own code, once Brian and I are
finished cleaning up.)

> >    2. we already have this issue on win32, and I don't like where we 
> are right now.
> >       Win32 is using msec, but we pass and return usec values.  That's 
> just sure
> >       to introduce bugs.  I'm going to cache *two* values in the 
> socket_t structure,
>
> >       s->timeout and s->timeout_ms.  This makes state changes quick, but
> >       keeps the data readily available for recall with getsockopt().
>
>And guarantees bugs.

Wrong.  These are internals.  NOT externals.  This particular value is set 
in one
place today (for sockets) ... apr_set_sockopt(...APR_SO_TIMEOUT...) ... and 
I will
assure it doesn't happen anywhere else.

>:-(  Caching speeds things up, but it is almost
>guaranteed to cause somebody to update 1 of them without updating the
>other at some point in the future.

Not when apr_socket_t is opaque.

>   Data duplication is almost never a good idea.

Except in opaque structures, such as private class members.
It's a terrific benefit of opaque types that we already use.

> > So unix -could- keep a tm handy if you need sec/usec as an argument,
> > and only set that value up once, each time we call timeout_set.  That's
> > a performance call I'm certain BPane will weight in on later.  But it's
> > totally internal and transparent to our library user.
>
>And has the same problem, that in the name of performance, you are going
>to introduce bugs.

No, only if the APR authors don't pay attention.  Which we do.  This is a 
reason
you don't do mass renames with perl scripts - you look at the code, 
discover any
existing bugs, and then apply the best new code.

> > >Either one of these solutions is most likely going to cause enough pain
> > >to the developer that APR's time code will be used rarely, IMHO.
> >
> > That's Roy's complaint already that we aren't using seconds.  And I
> > think (so far) we've proven that point incorrect.
> >
> > You are arguing that our usec times made sense, others argue that they
> > really didn't.  We are now suggesting 'Trust Us - Use Macros - Kill Bugs'.
> > Since the macros are completely processed at compile time for const
> > values, they are the best solution.
>
>I am simply suggesting nobody has weighed in from the user's point of
>view, other than to say "We must have usec resolution".  The API that is
>currently being discussed is big and complex.  I would personally never
>write a program using APR if a concept as simple as time required this
>much work to get into my program.  This API would literally stop me from
>using APR at all, because of how often I would need to read the docs on
>how to use the API.

If this is too complex, you better quit using apr_socket's, apr_file's, and
a whole lot more complicated types with lots 'o functions.

>How often do the programmers on this list look at the docs for simple
>functions, like poll or read/write.  I'm willing to bet almost never,
>because the APIs were designed so that they made sense once you learned
>them.  The time API that is being designed doesn't make sense to me as a
>programmer, which means that I will spend most of my time reading the
>docs on how to use it, which means I am unlikely to use it or any of
>APR, because time goes throughout APR.

I regularly pop up the .h files for the API's I don't recall off the top of 
my head
[why is it apr_pool_t* the first arg of this fn, but the last of that 
one?]  Please
don't claim that the rest of APR is simply simple.  And you have no business
claiming that the handful of macros are that hard to remember either.

>No disrespect intended at all here, but the API being designed reminds
>me very much of the native functions on Windows.  Ten ways to do
>everything, and all of them require far too much work to understand.

Ten ways to do what?  There is one [recommended] way.  If folks hack
around that, it's on them [just like if you do simple things using the NT
kernel calls instead of the public API.]

But sometimes you have to [use kernel calls] when the public API won't
support it.  This type is opaque, it won't be that difficult.

> > >Maybe would could have had:
> > >
> > >#ifndef NEW_TIME_FMT
> > >#error "Please convert you code to use busec's
> > >#endif
> >
> > Interesting option :-)  But not entirely practical, I think.
> > It doesn't take into account the developer's own header files
> > that might contain time constants.
>
>Sure it does.  It is just saying that if you include the apr_time.h
>header, you will have to define an APR specific macro to prove that you
>are using the new definition of apr_time_t.

I didn't dismiss it, just suggesting it's a pretty obtuse versioning scheme.
How many will we end up with like this?

Better to let the coder #define APR_VERSION_REQUIRED and provide
the warning if those don't match.  Much more generally applicable, and
it provides effectively the same solution.

I don't mean to sound harsh here, but we've been working in this direction
for three weeks or so, much longer than the current debate.  I'm just asking
myself where you've been while Brian Pane and I were introducing the new
macros and fixing up the code in preparation for initial profiling and 
whatever
potential change.

Bill



Mime
View raw message