Bug 29148 – replace platform time code with WTF::currentTime()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Yong Li]

We have WTF::currentTime() now. We should use it to replace those duplicate platform-dependent code.

1. In TimeoutChecker.cpp: getCPUTime currently returns system time except for PLATFORM(DARWIN) and PLATFORM(WIN_OS) (they can get actual thread time)

2. jsc.cpp: a lot of time related code can be simplified with currentTime()

Patch is coming

[Yong Li]

Created attachment 39385 [details]
the patch

the patch

[Ariya Hidayat]

Comment on attachment 39385 [details]
the patch

CurrentTime.h is missing in the patch.

[Yong Li]

(In reply to comment #2)
> (From update of attachment 39385 [details])
> CurrentTime.h is missing in the patch.

What do you mean? CurrentTime.h already exists in JavaScriptCore/wtf.

[Joe Mason]

I believe that in the current codebase, currentTime() is only ever used to check relative times, even though the descriptions specifies that it should return seconds since the Unix epoch.  That could let ports take shortcuts by implementing it to return seconds since some other epoch, or since the system boot, or whatever's easiest for them.  

I don't know if any ports actually DO take such shortcuts, but this should be double-checked if this patch is landed.

I was about to suggest renaming currentTime and changing the description to make it clear the way it was actually used, but I like this approach better.  No need to duplicate code.

[Adam Barth]

Comment on attachment 39385 [details]
the patch

I'm not an expert here, but this seems reasonable and this patch has been setting in the review queue for a while with little discussion.

[Eric Seidel (no email)]

Looks reasonable to me too.  CCing Darin and Maciej who I believe worked on this code long ago, so that they at least see this go by.

[WebKit Commit Bot]

Comment on attachment 39385 [details]
the patch

Rejecting patch 39385 from commit-queue.

This patch will require manual commit. Failed to run "['git', 'svn', 'dcommit']"  exit_code: 1  cwd: None

[Eric Seidel (no email)]

Comment on attachment 39385 [details]
the patch

Sigh.  Bug 28316, sorry.

[WebKit Commit Bot]

Comment on attachment 39385 [details]
the patch

Clearing flags on attachment: 39385

Committed r48736: <http://trac.webkit.org/changeset/48736>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.