722345 - remove request API

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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.

Comment 2

[Andrew McCreight [:mccr8]]

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.

Comment 3

[Luke Wagner [:luke]]

Any plans for this once bug 730234 is out of the way?

Comment 4

[Igor Bukanov]

(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.

Comment 5

[Luke Wagner [:luke]]

Attachment: old WIP

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.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 722345 part 1 - Remove now unused activity callback API. r=luke

Depends on D4085

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 722345 part 2 - Remove AutoCheckRequestDepth, rename CHECK_REQUEST to CHECK_THREAD. r=luke

Depends on D4422

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 722345 part 3 - Remove request API. r=luke

Depends on D4423

Comment 11

[Luke Wagner [:luke]]

Comment on attachment 9004484 [details]
Bug 722345 part 1 - Remove now unused activity callback API. r=luke

Luke Wagner [:luke] has approved the revision.

Comment 12

[Luke Wagner [:luke]]

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.

Comment 13

[Luke Wagner [:luke]]

Comment on attachment 9004486 [details]
Bug 722345 part 3 - Remove request API. r=luke

Luke Wagner [:luke] has approved the revision.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/6f17ffaad886
https://hg.mozilla.org/mozilla-central/rev/8542dc7212b4
https://hg.mozilla.org/mozilla-central/rev/e951ad8147a7

Comment 16

[Florian Scholz (Open Web Docs)]

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).