Bug 89236 - [BlackBerry] Put platform-specific GC policy in GCActivityCallback
Summary: [BlackBerry] Put platform-specific GC policy in GCActivityCallback
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-15 11:49 PDT by Yong Li
Modified: 2012-06-15 15:32 PDT (History)
4 users (show)

See Also:


Attachments
the patch (4.46 KB, patch)
2012-06-15 12:23 PDT, Yong Li
no flags Details | Formatted Diff | Diff
fix the style error (4.46 KB, patch)
2012-06-15 12:41 PDT, Yong Li
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2012-06-15 11:49:13 PDT
to add a GCActivityCallbackBlackBerry.cpp, and put our low memory GC policy into didAllocate().
Comment 1 Yong Li 2012-06-15 12:23:59 PDT
Created attachment 147874 [details]
the patch

Geoffrey, the patch adds one line to Heap.h:

bool isSafeToCollect() const { return m_isSafeToCollect; }

Do you think it is OK?
Comment 2 WebKit Review Bot 2012-06-15 12:28:46 PDT
Attachment 147874 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/runtime/GCActivityCallbackBlackBerry.cpp:22:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/GCActivityCallbackBlackBerry.cpp:28:  Missing space inside { }.  [whitespace/braces] [5]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Yong Li 2012-06-15 12:41:47 PDT
Created attachment 147878 [details]
fix the style error
Comment 4 Rob Buis 2012-06-15 14:35:09 PDT
Comment on attachment 147878 [details]
fix the style error

Looks good.
Comment 5 Geoffrey Garen 2012-06-15 14:44:00 PDT
> bool isSafeToCollect() const { return m_isSafeToCollect; }

This is fine.

Seriously, though, you're making GC on Blackberry O(n^2).
Comment 6 Yong Li 2012-06-15 14:47:34 PDT
(In reply to comment #5)
> > bool isSafeToCollect() const { return m_isSafeToCollect; }
> 
> This is fine.
> 
> Seriously, though, you're making GC on Blackberry O(n^2).

It is still using a limit which is just the small one (1MB).

I commented the isMemoryLow check and tested it on some heavy sites. It is a bit slower, but seems still OK.
Comment 7 WebKit Review Bot 2012-06-15 15:32:09 PDT
Comment on attachment 147878 [details]
fix the style error

Clearing flags on attachment: 147878

Committed r120502: <http://trac.webkit.org/changeset/120502>
Comment 8 WebKit Review Bot 2012-06-15 15:32:14 PDT
All reviewed patches have been landed.  Closing bug.