Bug 89481

Summary: [BlackBerry] GC in didAllocate() isn't safe
Product: WebKit Reporter: Yong Li <yong.li.webkit>
Component: WebKit BlackBerryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED LATER    
Severity: Normal CC: fpizlo, ggaren, oliver, rwlbuis, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch
none
fix style ggaren: review-

Yong Li
Reported 2012-06-19 09:51:09 PDT
Not all calls to Heap::didAllocate() are done in operation Allocation state. We need to do it in another way
Attachments
proposed patch (2.96 KB, patch)
2012-06-20 08:51 PDT, Yong Li
no flags
fix style (2.96 KB, patch)
2012-07-04 06:57 PDT, Yong Li
ggaren: review-
Yong Li
Comment 1 2012-06-20 08:51:39 PDT
Created attachment 148578 [details] proposed patch I know this is a bit ugly. Any suggestions about how to hack this neatly? For example, can I add an argument to GCActivityCallback::didAllocate() indicating it is safe to trigger GC?
WebKit Review Bot
Comment 2 2012-06-20 08:53:54 PDT
Attachment 148578 [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/GCActivityCallback.h:53: Missing space inside { }. [whitespace/braces] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yong Li
Comment 3 2012-07-04 06:57:59 PDT
Created attachment 150786 [details] fix style
Oliver Hunt
Comment 4 2012-07-05 13:24:58 PDT
Comment on attachment 150786 [details] fix style Why is this necessary?
Yong Li
Comment 5 2012-07-05 13:52:37 PDT
(In reply to comment #4) > (From update of attachment 150786 [details]) > Why is this necessary? This is a follow-up of https://bugs.webkit.org/show_bug.cgi?id=89236. The initial idea is use small heap size when system is under memory pressure. I ended up with putting platform-specific GC policy in GCActivityCallback. However I found it isn't safe to do GC in didAllocate. The only place it makes sense and is safe to collect garbage is reportExtraMemoryCost.
Rob Buis
Comment 6 2012-07-12 15:09:41 PDT
Any JSC folks want to review this?
Geoffrey Garen
Comment 7 2012-07-12 16:14:09 PDT
This looks like a design flaw in bug 89236 more than anything else. We don't want to modify cross-platform logic to fix this, since we don't want the behavior in bug 89236 in the first place.
Note You need to log in before you can comment on or make changes to this bug.