Bug 151656 - [EFL] Implement scheduleTimer and cancelTimer in IncrementalSweeper class
Summary: [EFL] Implement scheduleTimer and cancelTimer in IncrementalSweeper class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-30 08:40 PST by Gyuyoung Kim
Modified: 2015-12-07 08:51 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.82 KB, patch)
2015-11-30 08:41 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (3.13 KB, patch)
2015-12-01 00:36 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2015-12-02 00:03 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch for landing (3.19 KB, patch)
2015-12-07 03:37 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.