Bug 151656

Summary: [EFL] Implement scheduleTimer and cancelTimer in IncrementalSweeper class
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: JavaScriptCoreAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff, ossy, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Gyuyoung Kim 2015-11-30 08:40:44 PST 
As mac and gtk ports, EFL ports supports IncremntalSweeper using Ecore_Timer.
Comment 1 Gyuyoung Kim 2015-11-30 08:41:51 PST 
Created attachment 266246 [details]
Patch
Comment 2 Gyuyoung Kim 2015-12-01 00:36:19 PST 
Created attachment 266340 [details]
Patch
Comment 3 Gyuyoung Kim 2015-12-01 05:34:36 PST 
Comment on attachment 266340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=266340&action=review

> Source/JavaScriptCore/heap/IncrementalSweeper.cpp:79
> +    if (m_isTimerFrozen) {

I couldn't find an EFL function to check if timer is frozen. If there is any function to check it, please let me know.
Comment 4 Gyuyoung Kim 2015-12-02 00:03:20 PST 
Created attachment 266434 [details]
Patch
Comment 5 Csaba Osztrogonác 2015-12-07 03:32:45 PST 
Comment on attachment 266434 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=266434&action=review

LGTM, r=me with some comments.

> Source/JavaScriptCore/heap/IncrementalSweeper.cpp:47
> -#if USE(CF) || (USE(GLIB) && !PLATFORM(EFL))
> +#if USE(CF) || USE(GLIB)

I know that USE(GLIB) is true for us too, but it is a little bit
confusing to rely on USE(GLIB) when we don't use GLIB timer here.

I suggest using "#if USE(CF) || PLATFORM(EFL) || USE(GLIB)".
It is redundant, but more descriptive and less confusing.

> Source/JavaScriptCore/heap/IncrementalSweeper.h:52
> -#if USE(CF) || (USE(GLIB) && !PLATFORM(EFL))
> +#if USE(CF) || USE(GLIB)

ditto
Comment 6 Gyuyoung Kim 2015-12-07 03:37:34 PST 
Created attachment 266766 [details]
Patch for landing
Comment 7 Gyuyoung Kim 2015-12-07 03:38:02 PST 
Comment on attachment 266434 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=266434&action=review

>> Source/JavaScriptCore/heap/IncrementalSweeper.cpp:47
>> +#if USE(CF) || USE(GLIB)
> 
> I know that USE(GLIB) is true for us too, but it is a little bit
> confusing to rely on USE(GLIB) when we don't use GLIB timer here.
> 
> I suggest using "#if USE(CF) || PLATFORM(EFL) || USE(GLIB)".
> It is redundant, but more descriptive and less confusing.

Agreed.
Comment 8 WebKit Commit Bot 2015-12-07 08:51:33 PST 
Comment on attachment 266766 [details]
Patch for landing

Clearing flags on attachment: 266766

Committed r193627: <http://trac.webkit.org/changeset/193627>
Comment 9 WebKit Commit Bot 2015-12-07 08:51:36 PST 
All reviewed patches have been landed.  Closing bug.