1558556 - JSContext needs to know when it's running on an XPCOM thread

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Kris Wright :KrisWright]

Since Spidermonkey doesn't know if it's running as a part of Firefox, JSContext can only tell it's on a helper thread if it's a native helper thread. It holds a pointer to its helper thread which it can use to check against protected data checks or to ensure it's running the helper thread friendly version of its jobs. JSContext needs some kind of flag to know when it's running on an XPCOM helper thread, and these checks need to be updated such that jobs can safely run offthread in both native spidermonkey threads and with XPCOM threads.

Comment 1

[Kris Wright :KrisWright]

To add from this morning, it looks like I'm going about this one of two ways unless there's a better solution:
a. Change null checks for Helperthread() to look for kind_ instead
https://searchfox.org/mozilla-central/rev/c606cdd6d014fee4034da1702d484c0d41b604c9/js/src/gc/Allocator.cpp#249 is a random example - there are a lot of places where JSContext looks to see if it's on a helperthread by null checking helperThread(). XPCOM contexts could just set kind_ and the null checks could look for the context kind rather than the thread itself.

b. Refactor offthread tasks to work without JSContext while running in XPCOM threads
Rather than teaching XPCOM threads to hold onto a JSContext, we could just run tasks in XPCOM that work around it. I'm not completely sure to what extent some of the tasks use JSContext and if it is possible (such as IonBuilder using ionCompilingSafeForMinorGC, etc...) so it might require significant refactoring.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Kristen Wright :KrisWright from comment #1)

b. Refactor offthread tasks to work without JSContext while running in XPCOM threads
Rather than teaching XPCOM threads to hold onto a JSContext, we could just run tasks in XPCOM that work around it. I'm not completely sure to what extent some of the tasks use JSContext and if it is possible (such as IonBuilder using ionCompilingSafeForMinorGC, etc...) so it might require significant refactoring.

This option isn't feasible, mostly because off-thread parsing uses JSContexts extensively right now. This will likely change in the future and some of the work is already in progress, but it's going to take time :)

Comment 3

[Kris Wright :KrisWright]

Attachment: Bug 1558556 - swap cx->helperThread() nullchecks to check for js::ContextKind::HelperThread

Added isHelperThreadContext() to check that cx runs on a helper thread, similar to isMainThreadContext(). Replaced null-checks meant to look for helper threads with isHelperThreadContext(), leaving instances where helperthread() is checked in order to access the actual helper thread.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5c1755178d9
swap cx->helperThread() nullchecks to check for js::ContextKind::HelperThread r=tcampbell

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a5c1755178d9