RESOLVED FIXED 126334
CStack: Introduce tracking of the top VMEntryScope.
https://bugs.webkit.org/show_bug.cgi?id=126334
Summary CStack: Introduce tracking of the top VMEntryScope.
Mark Lam
Reported 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.
Attachments
the patch. (4.86 KB, patch)
2013-12-30 23:16 PST, Mark Lam
ggaren: review+
Mark Lam
Comment 1 2013-12-30 23:16:37 PST
Created attachment 220151 [details] the patch.
Mark Lam
Comment 2 2013-12-30 23:19:39 PST
Landed in r161174 on the jsCStack: <http://trac.webkit.org/r161174>.
Geoffrey Garen
Comment 3 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
Mark Lam
Comment 4 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.
Note You need to log in before you can comment on or make changes to this bug.