Bug 170725

Summary:

[WebCore][JSC] ResourceUsageData.{timeOfNextEdenCollection,timeOfNextFullCollection} should be MonotonicTime

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

WebCore Misc.

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, buildbot, cdumez, cgarcia, cmarcelo, dbates, keith_miller, mark.lam, msaboff, ryanhaddad, saam, sam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	sam: review+

Yusuke Suzuki

Reported 2017-04-11 04:25:48 PDT

...

Attachments
Patch (13.16 KB, patch) 2017-04-11 09:18 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (13.24 KB, patch) 2017-04-11 09:19 PDT, Yusuke Suzuki	sam: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2017-04-11 09:18:25 PDT

Created attachment 306822 [details] Patch

Yusuke Suzuki

Comment 2 2017-04-11 09:19:15 PDT

Created attachment 306823 [details] Patch

Yusuke Suzuki

Comment 3 2017-04-11 09:22:48 PDT

Comment on attachment 306823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306823&action=review > Source/WebCore/page/cocoa/ResourceUsageOverlayCocoa.mm:481 > + MonotonicTime now = MonotonicTime::now(); Inside the timer, we usually use MonotonicTime. Thus, using MonotonicTime to calculate the difference should be appropriate instead of WallTime.

Sam Weinig

Comment 4 2017-04-11 10:39:41 PDT

Comment on attachment 306823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306823&action=review > Source/WTF/wtf/MonotonicTime.h:49 > + constexpr MonotonicTime() { } Want to make a few tests that test constexpr MonotonicTime?

Yusuke Suzuki

Comment 5 2017-04-11 11:09:13 PDT

Comment on attachment 306823 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306823&action=review Thanks! > Source/JavaScriptCore/heap/GCActivityCallback.cpp:81 > + return MonotonicTime::fromRawSeconds(CFRunLoopTimerGetNextFireDate(m_timer.get())); Oops, I need to change this thing to MonotonicTime::now() + (CFRunLoopTimerGetNextFireDate(m_timer.get()) - CFAbsoluteTimeGetCurrent()). >> Source/WTF/wtf/MonotonicTime.h:49 >> + constexpr MonotonicTime() { } > > Want to make a few tests that test constexpr MonotonicTime? Added.

Yusuke Suzuki

Comment 6 2017-04-11 11:25:48 PDT

Committed r215241: <http://trac.webkit.org/changeset/215241>

Ryan Haddad

Comment 7 2017-04-11 11:34:21 PDT

(In reply to Yusuke Suzuki from comment #6) > Committed r215241: <http://trac.webkit.org/changeset/215241> This change broke macOS and iOS builds: https://build.webkit.org/builders/Apple%20Sierra%20Release%20%28Build%29/builds/694 https://build.webkit.org/builders/Apple%20iOS%2010%20Release%20%28Build%29/builds/543

Yusuke Suzuki

Comment 8 2017-04-11 11:39:15 PDT

Committed r215244: <http://trac.webkit.org/changeset/215244>

Note You need to log in before you can comment on or make changes to this bug.