This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [ping2][PATCH v2][BZ #12515] Improve precision of clock function
- From: Rich Felker <dalias at aerifal dot cx>
- To: Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: Siddhesh Poyarekar <siddhesh at redhat dot com>, libc-alpha at sourceware dot org
- Date: Mon, 10 Jun 2013 21:40:28 -0400
- Subject: Re: [ping2][PATCH v2][BZ #12515] Improve precision of clock function
- References: <20130521145611 dot GM8927 at spoyarek dot pnq dot redhat dot com> <20130521151839 dot GA18430 at domone dot kolej dot mff dot cuni dot cz> <20130521153442 dot GO8927 at spoyarek dot pnq dot redhat dot com> <519B9A09 dot 6030305 at cs dot ucla dot edu> <20130521161441 dot GQ8927 at spoyarek dot pnq dot redhat dot com> <20130603092604 dot GL2145 at spoyarek dot pnq dot redhat dot com> <20130610083821 dot GD1570 at spoyarek dot pnq dot redhat dot com> <51B66522 dot 1060008 at cs dot ucla dot edu>
On Mon, Jun 10, 2013 at 04:45:38PM -0700, Paul Eggert wrote:
> On 06/10/13 01:38, Siddhesh Poyarekar wrote:
>
> >>> + if (__clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &ts) == 0)
> >>> + return (ts.tv_sec * CLOCKS_PER_SEC
> >>> + + ts.tv_nsec / (1000000000 / CLOCKS_PER_SEC));
>
> Sorry, now that I'm thinking about it, I thought of two more
> things.
>
> First, the above code relies on wraparound arithmetic after
> signed integer overflow, assuming time_t is signed, but GCC
> doesn't guarantee this by default. So, should
The old code with times() did too.
> the function containing the above code be preceded by a pragma,
> like this?
>
> #pragma GCC optimize ("wrapv")
No. There are actually two issues. First, to make the code work as-is
with the wrapping behavior, the values should just be converted to the
appropriate unsigned types so that the wrapping is well-defined, then
converted, either via a cast or implicitly, back to the right type.
This conversion takes place in an implementation-defined way, which is
always modular reduction to the range of the destination type on
compilers that glibc supports.
The second issue is that the wrapping is non-conforming, which I see
you've addressed below, but this should be addressed separately from
fixing the poor precision.
> Second, 'clock' is weird since POSIX specifically
> mentions wraparound as an allowed behavior, whereas C11
> says that wraparound is not allowed. POSIX will be changed
> to match C11; see:
>
> http://austingroupbugs.net/bug_view_page.php?bug_id=686
>
> and also see:
>
> http://austingroupbugs.net/bug_view_page.php?bug_id=703
And http://sourceware.org/bugzilla/show_bug.cgi?id=15524 and the
somewhat related http://sourceware.org/bugzilla/show_bug.cgi?id=13080.
> Are we deferring the second issue for now?
> If so, we should probably put in a comment about it,
> since it'll come up again eventually....
Either way, unrelated changes should not be made in the same commit, I
think. Whether such an unrelated comment is appropriate as part of the
commit is beyond the scope of my understanding of glibc policy, but my
feeling would be that such a comment should be added separately, if at
all, rather than as part of this fix. Ideally the issue would fixed
right away without the need to keep a comment around, but I'm not sure
how popular this change will be, and it will almost certainly require
a new symbol version to make it past review...
Rich