Closed Bug 722345 Opened 8 years ago Closed Last year

remove request API

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: igor, Assigned: jandem)

References

Details

Attachments

(3 files, 1 obsolete file)

The bug 675078 made the runtime single-threaded but it kept the request API as they are used for the following things besides thread-synchronization:

1. The GC scans the native stack only if the calling thread was in active or suspended request.

2. The transition no_request -> request wakes up the watchdog thread that will terminate the long-running script if the the main thread spends too much time in the request.

3. The cycle collector treats JSContext instances as black if the context is in the request.

We should consider how to address the corresponding needs without the notion of requests so the API could be eliminated. 

For example, until the conservative GC is disabled, we can always scan the calling thread stack tolerating potential false positives. The watchdog case can try to observe the script invocation and base its decisions on that.  And cycle collectors it seems can decide about active context based on the color of the global object stored in it or similar.
Depends on: 722348
Duplicate of this bug: 719858
I don't really know what outstanding requests are, but as far as the cycle collector goes, what is happening is that if there are any outstanding requests, then a JSContext will keep its global object alive, even if the global object isn't marked black.  Right now, if the global stored is black, then the JSContext won't keep anything additional alive, even with outstanding requests.  Well, aside from itself.
Depends on: 723021
Depends on: 723286
Depends on: 731618
Any plans for this once bug 730234 is out of the way?
(In reply to Luke Wagner [:luke] from comment #3)
> Any plans for this once bug 730234 is out of the way?

We need one more blocker here besides that bug and 720045. Currently the cycle collector uses in non-trivial way the contexts entered the requests as a strong roots for the global object stored in them. To address that we either need one global per compartment when entered compartments will serve as roots. Or, if that takes long time, we can remove the global from JSContext and store them as strong roots in the AutoEnterCompartment structure.
Depends on: 730234
Depends on: 734250
Depends on: 734255
No longer depends on: 734255
Depends on: 792235
Depends on: 792237
Attached patch old WIP (obsolete) — Splinter Review
This patch is really old and I'm not working on it, but I thought I should post it b/c it shows how (I think) to replace the remaining vestigial uses of requests with entering/leaving compartments.
Assignee: general → nobody
Duplicate of this bug: 1065577
Duplicate of this bug: 1267297
Depends on: 1267297
Depends on D4423
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #745210 - Attachment is obsolete: true
Comment on attachment 9004484 [details]
Bug 722345 part 1 - Remove now unused activity callback API. r=luke

Luke Wagner [:luke] has approved the revision.
Attachment #9004484 - Flags: review+
Comment on attachment 9004485 [details]
Bug 722345 part 2 - Remove AutoCheckRequestDepth, rename CHECK_REQUEST to CHECK_THREAD. r=luke

Luke Wagner [:luke] has approved the revision.
Attachment #9004485 - Flags: review+
Comment on attachment 9004486 [details]
Bug 722345 part 3 - Remove request API. r=luke

Luke Wagner [:luke] has approved the revision.
Attachment #9004486 - Flags: review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f17ffaad886
part 1 - Remove now unused activity callback API. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/8542dc7212b4
part 2 - Remove AutoCheckRequestDepth, rename CHECK_REQUEST to CHECK_THREAD. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/e951ad8147a7
part 3 - Remove request API. r=luke
Doesn't look like this affects MDN Web Docs. Removing ddn.
(if you disagree, please explain what kinds of MDN doc updates would be needed).
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.