Boost logo

Boost :

Subject: Re: [boost] [chrono] Possible bug in boost::chrono::process_real_cpu_clock::now() under Windows 32bits
From: ivan.lelann_at_[hidden]
Date: 2012-01-05 12:31:24


----- Mail original -----
> De: "Vicente J. Botet Escriba" <vicente.botet_at_[hidden]>
> À: boost_at_[hidden]
> Envoyé: Jeudi 5 Janvier 2012 17:53:12
> Objet: Re: [boost] [chrono] Possible bug in boost::chrono::process_real_cpu_clock::now() under Windows 32bits
>
> Le 05/01/12 17:38, Howard Hinnant a écrit :
> > On Jan 5, 2012, at 10:30 AM, ivan.lelann_at_[hidden] wrote:
> >
> >> Hi,
> >>
> >> As of 1.48 Boost.Chrono contains code below for
> >> boost::chrono::process_real_cpu_clock::now()
> >> (boost\chrono\detail\inlined\win\process_cpu_clocks.hpp)
> >>
> >> clock_t c = ::clock();
> >> /* ... */
> >> return time_point(
> >> duration(c*(1000000000l/CLOCKS_PER_SEC))
> >> );
> >>
> >> duration::rep is int64/nanoseconds, clock_t is long. This is
> >> VS2008, Win XP 32bits.
> >>
> >> I think "c" should be cast to duration::rep before being
> >> multiplied.
> >> I randomly get negative time_point and this seems to be fixed by
> >> replacing
> >> "c" with "((duration::rep) (c))" in duration constructor :
> >>
> >> duration( ((duration::rep) (c))
> >> *(1000000000l/CLOCKS_PER_SEC))
> >>
> >> Can someone confirm my view and that the fix is correct ?
> > Your fix looks correct to me. I'm not positive what value
> > CLOCKS_PER_SEC has on your platform. But here is a reformulation
> > of your solution that might be a little safer (depending on what
> > CLOCKS_PER_SEC actually is):
> >
> > typedef ratio_divide<giga, ratio<CLOCKS_PER_SEC>>::type R;
> > return time_point(
> > duration(static_cast<rep>(c)*R::num/R::den)
> > );
> >
> > Using giga avoids mistakes in counting zeros.
> >
> > Using ratio_divide reduces the fraction 1000000000l/CLOCKS_PER_SEC
> > to lowest terms, no matter the value of CLOCKS_PER_SEC.
> >
> > You can use rep as a shortcut for duration::rep, assuming this is
> > inside of the clock's now() function.
> >
> > static_cast<rep> is a little safer than a C cast. I would expect
> > the results to be the same, but it is good to code defensively.
> >
> > If you know a-priori that R::den is 1, you might:
> >
> > typedef ratio_divide<giga, ratio<CLOCKS_PER_SEC>>::type R;
> > static_assert(R::den == 1, "oops!");
> > return time_point(
> > duration(static_cast<rep>(c)*R::num)
> > );
>
> Hi,
>
> yes this seems much more safer.
>
> Ivan please, could you create a ticket for this issue?
>

Done :
https://svn.boost.org/trac/boost/ticket/6361

Regards,
Ivan


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk