Use uint in ProfLazyMode test when measuring time delta.

Use uint in ProfLazyMode test when measuring time delta.

While testing ProfLazyMode stability I encountered a situation when the cycle supposed to run for 200 ms started to run "infinitely" because delta between two int64_t values became negative.

Committed: http://code.google.com/p/v8/source/detail?r=2078

[Søren Thygesen Gjesse, 2009-05-29 09:25:12]

LGTM

[William Hesse, 2009-05-29 12:19:49]

http://codereview.chromium.org/115918/diff/1/2
File test/cctest/test-log.cc (right):

http://codereview.chromium.org/115918/diff/1/2#newcode173
Line 173: while ((delta = i::OS::Ticks() - started_us) < 200 * 1000) {
The condition should really be
(ended_us = ticks()) < started_us + 200 * 1000)
delta_us = ended_us - started_us;
in order to correctly compare unsigneds, because the difference of two unsigneds
cannot be negative.

[William Hesse, 2009-05-29 12:55:32]

http://codereview.chromium.org/115918/diff/1/2
File test/cctest/test-log.cc (right):

http://codereview.chromium.org/115918/diff/1/2#newcode173
Line 173: while ((delta = i::OS::Ticks() - started_us) < 200 * 1000) {
On 2009/05/29 12:19:49, William Hesse wrote:
> The condition should really be
> (ended_us = ticks()) < started_us + 200 * 1000)
> delta_us = ended_us - started_us;
> in order to correctly compare unsigneds, because the difference of two
unsigneds
> cannot be negative.
> 
> 

Actually, we don't really understand what is going on.  I think converting to
unsigned is the wrong idea, though.  OS::Ticks should not be anywhere near the
negative range, and tv_secs and tv_usecs (used in GetTimeOfDay, used by
OS::Ticks) should also be safe.

I think the best practice is to put two times you want to compare on opposite
sides of the comparison operator, rather than subtracting them.  It should be
fine to leave them as ints, too.

if (end_time < start_time + desired delta)
is valid as long as end_time and start_time + desired delta
are in the valid range of the type.

But I don't see how the change to uint is supposed to help.

[Tobias Kaes, 2009-05-29 13:12:16]

I don't think either is really correct. The implementation of OS::Tick can wrap
around ways before reaching the int64 or uint64 limit because its implementation
eg. on windows has a max value of uint32*1000 and then starts over with zero

So when started_us is before and ended_us after an OS::Tick wrap-around you will
have problems. Using an uint64 at least avoids the infinite loop you get with
int64, but it can result in terminating too early (ie. before 200ms are over).