Switch to using TimeTicks rather than Time in message loops

Switch to using TimeTicks rather than Time in message loops

Switch to using TimeTicks rather than Time so that we
are not dependent on changes in the system clock.

r=mbelshe,darin

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65322

[jar (doing other things), 2010-11-05 18:41:42]

FYI: I'll get this to run on try servers (again) before landing.  It was
running... but the LKGB is out-of-date, and giving me merge conflicts <sigh>

http://codereview.chromium.org/3884001/diff/110001/111001
File base/message_loop.cc (left):

http://codereview.chromium.org/3884001/diff/110001/111001#oldcode556
base/message_loop.cc:556: if (!delayed_work_queue_.empty())
Note that when delayed work queue is empty, perhaps we should be setting
next_delayed_work_time to something.  Alternatively, perhaps we should do this
check after handling DeferOrRunPendingTask.

That semantic change should probably be isolated to as separte CL.

[darin (slow to review), 2010-11-05 20:21:08]

On Fri, Nov 5, 2010 at 11:41 AM, <jar@chromium.org> wrote:

> Reviewers: darin,
>
> Message:
> FYI: I'll get this to run on try servers (again) before landing.  It was
> running... but the LKGB is out-of-date, and giving me merge conflicts
> <sigh>
>

gcl try takes a -r argument that can be used to specify the revision you
want the
try servers to test against.

-darin



>
>
>
> http://codereview.chromium.org/3884001/diff/110001/111001
> File base/message_loop.cc (left):
>
> http://codereview.chromium.org/3884001/diff/110001/111001#oldcode556
> base/message_loop.cc:556: if (!delayed_work_queue_.empty())
> Note that when delayed work queue is empty, perhaps we should be setting
> next_delayed_work_time to something.  Alternatively, perhaps we should
> do this check after handling DeferOrRunPendingTask.
>
> That semantic change should probably be isolated to as separte CL.
>
> Description:
> Switch to using TimeTicks rather than Time in message loops
>
> Switch to using TimeTicks rather than Time so that we
> are not dependent on changes in the system clock.
>
> r=mbelshe,darin
>
> Please review this at http://codereview.chromium.org/3884001/show
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>  M     base/message_loop.h
>  M     base/message_loop.cc
>  M     base/message_pump.h
>  M     base/message_pump_default.h
>  M     base/message_pump_default.cc
>  M     base/message_pump_glib.h
>  M     base/message_pump_glib.cc
>  M     base/message_pump_libevent.h
>  M     base/message_pump_libevent.cc
>  M     base/message_pump_mac.h
>  M     base/message_pump_mac.mm
>  M     base/message_pump_win.h
>  M     base/message_pump_win.cc
>  M     remoting/jingle_glue/jingle_thread.cc
>
>
>

[darin (slow to review), 2010-11-05 20:37:59]

LGTM

http://codereview.chromium.org/3884001/diff/141003/175011
File base/message_pump_mac.mm (right):

http://codereview.chromium.org/3884001/diff/141003/175011#newcode229
base/message_pump_mac.mm:229: // TODO(jar): We may need a more efficient way to
go between these times, but
You should ask Mark Mentovai to look at this.  He might have some good ideas for
improvements here.  Perhaps there are some other Mac APIs that we can leverage.

[jar (doing other things), 2010-11-05 21:18:05]

Mark,

I added a TODO() in the message_pump_mac.mm file, as this change is not
especially efficient translating from a TimeTicks:: basis to a Time:: basis (as
seems to be needed on the Mac).  Darin suggested that perhaps you'd have
suggestions about this.

I can land the change at night, and watch to see if there is a measurable perf
impact on the Mac.  On Windows, calls to Now() can be a bit expensive, at a
minimum because they take a lock(), but I'm not (yet) well informed about the
cost of these calls on a Mac.

Comments?

Thanks,

Jim

[Mark Mentovai, 2010-11-05 21:58:07]

I don’t think it’s called frequently enough to make a difference in
perf measurements, but let’s get some data to be sure.