> From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED] > > At 03:21 PM 7/14/2002, Ryan Bloom wrote: > > >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.
The macros are no faster than the constant. The implementation is faster. Completely different. > >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. BTW, nsec is completely bogus. There isn't an OS that I know of that reports nsec intervals. Windows comes the closest with 100 nsec chunks, but that isn't nsec. Having a macro that purports to report nsecs when NONE of our OSes can accurately report them is bogus. Four macros, by four different scales, equals 16 macros. And, at the end of the day, the values stored are useful once you leave APR. BTW, the apr_time_xmake(sec, xsec) macro is completely wrong IMHO. In order to use it, I need to provide multiple values, but that doesn't make any sense, because we have four time scales. Either provide a single value, or four. Two just doesn't make sense. > 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?) Perhaps an example of the difference, because I don't see it. > > > >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. Quite honestly, I have tried to ignore the macro-ization that has been done recently, although whenever I see ten commits in a row to implement them, I do curse loudly. I would never use those macros with today's implementation, because they just get in the way of me knowing what is going on. > >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.) You're right, it is no more difficult, it is equally as difficult. My complaint is that the concept of time is FAR simpler than file or network operations. I don't mind complex API's for complex problems, but time is a simple complex. It is an offset from a set point in time, or an offset from an arbitrary point in time. Why is such a simple concept causing such a complex solution? > > > 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. If it is only happening in one place, then only store it in one format. I don't care if it is internals or externals either. Data duplication is a bad idea unless the performance improvement is massive. Every other place we have had this kind of data duplication in APR before, it has caused bugs. And yes, the original implementation for APR's time code had this kind of duplication on Windows to solve the exact problem that you are trying to solve. > >:-( 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. It does the day that you don't pay attention to a commit because you are busy, and a new developer makes a change to the code. Someday, you will either move on to a new project or not review a commit, and a bug will be committed. I am willing to stake my reputation on it. (Not sure what that is worth today, but there it is). > >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. Funny, but we have done multiple mass-renames with Perl code in the past. > >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. Again, complex solutions for complex problems, simple solutions for simple problems. We have just designed an incredibly complex solution for a very simple problem. > > > >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? How many times will we have vetos based on type names? Obviously the macro wouldn't last forever, just long enough to resolve the veto. It is a technical hack to get around a veto, nothing more. > 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. For the most part, I have been busy and not watching too closely. When the macros were added, I assumed I could ignore them. Now I can't, because they are the only way that the time code can work. Ryan
