Bug 126334

Summary: CStack: Introduce tracking of the top VMEntryScope.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 126320    
Attachments:
Description Flags
the patch. ggaren: review+

Description Mark Lam 2013-12-30 23:09:37 PST
When we start measuring the stack usage of each VMEntryScope, we'll need to know which VMEntryScope is the top (most recent) one, not just the first one.  Also, for correctness, when we set a new jsStackLimit, we should set it on the top VMEntryScope, and not on the first (oldest) one.  That is because the 2 scopes may be on 2 different thread stacks, and the most present stack limits only apply to the most recent scope.  That said, presently, VMEntryScope::updateStackLimits() does not rely on any scope specific data.  So, the old code calling updateStackLimits() on the oldest VMEntryScope hasn't manifested any issues yet.

This is a step towards https://bugs.webkit.org/show_bug.cgi?id=126320.
Comment 1 Mark Lam 2013-12-30 23:16:37 PST
Created attachment 220151 [details]
the patch.
Comment 2 Mark Lam 2013-12-30 23:19:39 PST
Landed in r161174 on the jsCStack: <http://trac.webkit.org/r161174>.
Comment 3 Geoffrey Garen 2014-01-02 14:04:27 PST
Comment on attachment 220151 [details]
the patch.

r=me on this patch, but I think the overall approach should change, in this way:

- VM should store the stack limits, as it does today
- VMEntryScope should set/reset the VM's stack limits, based on the current thread's stack limit (and the JSStack size limit if using the CLoop)
- ErrorHandlingMode should be renamed to ErrorHandlingScope, and it should set/reset the VM's stack limits just like VMEntryScope does, but with a more lenient limit

That means:
- Entering an error handling scope shouldn't require a callback to VMEntryScope
- Entering an error handling scope shouldn't require a callback to JSStack
- JSStack should have an error handling mode
- JSStack::enable/disableErrorSTackReserve should not exist
- topEntryScope and prevTopEntryScope should not exist
Comment 4 Mark Lam 2014-01-02 15:19:44 PST
Thanks for the revieww.  Status updated in r161232: <http://trac.webkit.org/r161232>.

Will address additional feedback in a separate bug.  topEntryScope and prevTopEntryScope are also needed for https://bugs.webkit.org/show_bug.cgi?id=126320 to compute the current JS stack usage.  So, I can't remove them.