Bug 126334 - CStack: Introduce tracking of the top VMEntryScope.
Summary: CStack: Introduce tracking of the top VMEntryScope.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks: 126320
  Show dependency treegraph
 
Reported: 2013-12-30 23:09 PST by Mark Lam
Modified: 2014-01-02 15:19 PST (History)
5 users (show)

See Also:


Attachments
the patch. (4.86 KB, patch)
2013-12-30 23:16 PST, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

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