Closed
Bug 551680
Opened 15 years ago
Closed 15 years ago
replacing JS_(Suspend|Resume)Request calls with JSAutoSuspendRequest
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: igor, Assigned: igor)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
Currently to implement a conservative stack scanning in the bug 516832 the GC must be able to suspend an arbitrary native thread which brings significant complexity to the implementation and constrains embeddings, since Linux does not have a native suspend support, see the bug 538702. This is necessary to support threads that calls JS_SuspendRequest.
On the other hand if a thread that suspends the request contains a single native frame that both calls JS_SuspendRequest and JS_ResumeRequest, then it is necessary to scan the native stack only up to that frame without the need to suspend the thread. Unfortunately the flexibility that separated JS_SuspendRequest and JS_ResumeRequest API provides means that the functions could be called from rather different native frames. So the idea is to replace calls to JS_SuspendRequest with JSAutoSuspendRequest and add a C API taking a callback like JS_PauseRequest(cx, callback). This will ensure that a frame that both suspends and resume exists.
Clearly, for compatibility with older code that would still call JS_SuspendRequest, the conservative stack scanner would need to suspend the thread. But at least that could be ifdefed.
Assignee | ||
Comment 1•15 years ago
|
||
The patch replaces most explicit JS_SuspendRequest and JS_ResumeRequest calls through out the browser and js shell with JSAutoSuspendRequest constructor/destructor. To support the case of the request transfer from one context to another it adds a new API, JS_TransferRequest(cx, another). That allows to avoid any locks while the transfer.
What the patch has not yet done is to eliminate the calls to XPCJSContextStack::Push. Fixing that would require to lift that call up to a few caller frames. That I would like to do that in a separated patch or bug to minimize the number of changes in this patch.
Attachment #431855 -
Flags: review?(mrbkap)
Assignee | ||
Comment 2•15 years ago
|
||
The new version insists that JS_TransferRequest is called for an context in request. It also changes EvalInContext from shell/js.cpp to use the new API for the evalcx context.
Attachment #431855 -
Attachment is obsolete: true
Attachment #432454 -
Flags: review?(mrbkap)
Attachment #431855 -
Flags: review?(mrbkap)
Updated•15 years ago
|
Attachment #432454 -
Flags: review?(mrbkap) → review+
Comment 3•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•