Closed
Bug 1031529
Opened 10 years ago
Closed 10 years ago
Remove JS_THREADSAFE and add a --no-threads shell flag
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: gkw, Assigned: jandem)
References
Details
Attachments
(4 files)
19.20 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
126.43 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
7.10 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
805 bytes,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
After an IRC conversation, it was agreed upon that we should replace JS_THREADSAFE with a shell flag, probably --disable-threads. (not --disable-threadsafe) One should note how this is related/not related to --ion-offthread-compile=[on|off] though. (The former is not ion-specific, the latter is ion-specific?) Bug 720018 and bug 859545 may or may not be related.
Flags: needinfo?(jdemooij)
Comment 1•10 years ago
|
||
--disable-threads should imply --ion-offthread-compile=off, but not vice versa.
Assignee | ||
Comment 2•10 years ago
|
||
Steve can we land bug 975011? Gary suggested we fix that bug first.
Flags: needinfo?(jdemooij) → needinfo?(sphink)
Assignee | ||
Comment 3•10 years ago
|
||
(I'd be happy to take this bug btw once that's fixed.)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Steve Fink [:sfink], PTO Jul 28-Aug 6 from comment #4) > Bug 975011 finally stuck. Excellent, taking this.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Comment 6•10 years ago
|
||
\o/
Assignee | ||
Updated•10 years ago
|
Summary: Removing JS_THREADSAFE, and add a --disable-threads shell flag → Remove JS_THREADSAFE and add a --no-threads shell flag
Assignee | ||
Comment 7•10 years ago
|
||
This patch adds a --no-threads shell flag to disable PJS threads and helper threads. The fuzzers can use this flag instead of --disable-threadsafe builds, to make problems easier to reproduce. Note that the shell watchdog thread, for timeout(), is still allowed: --disable-threadsafe builds currently use SIGALRM, that's not more deterministic than the watchdog thread.
Attachment #8460826 -
Flags: review?(bhackett1024)
Assignee | ||
Comment 8•10 years ago
|
||
This patch gets rid of the JS_THREADSAFE defines everywhere. 45 files changed, 53 insertions(+), 777 deletions(-)
Attachment #8460827 -
Flags: review?(bhackett1024)
Assignee | ||
Updated•10 years ago
|
Attachment #8460827 -
Attachment description: Part 2 - rm JS_THREADSAFE defines → Part 2 - rm JS_THREADSAFE ifdefs
Assignee | ||
Comment 9•10 years ago
|
||
All builds will be threadsafe, so this patch stops defining JS_THREADSAFE (previous patch removed all uses) and removes the --disable-threadsafe code.
Attachment #8460828 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8460831 -
Flags: review?(bhackett1024)
Comment 11•10 years ago
|
||
Comment on attachment 8460826 [details] [diff] [review] Part 1 - Add shell flag Review of attachment 8460826 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/HelperThreads.cpp @@ +422,5 @@ > > if (threads) > return; > > + MOZ_ASSERT(CanUseExtraThreads()); maybe move this assert further up?
Attachment #8460826 -
Flags: review?(bhackett1024) → review+
Comment 12•10 years ago
|
||
Comment on attachment 8460827 [details] [diff] [review] Part 2 - rm JS_THREADSAFE ifdefs Review of attachment 8460827 [details] [diff] [review]: ----------------------------------------------------------------- Yay! Can you remove JS_ION next?
Attachment #8460827 -
Flags: review?(bhackett1024) → review+
Updated•10 years ago
|
Attachment #8460831 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Brian Hackett (:bhackett) from comment #12) > Yay! Can you remove JS_ION next? Unfortunately people are using SM on platforms for which we don't have a JIT, and I think it'd be nice to support this. Maybe we could move more code outside JS_ION though...
Assignee | ||
Comment 14•10 years ago
|
||
Green on Try, FWIW: https://tbpl.mozilla.org/?tree=Try&rev=8d97a21e435d (There's some check_spidermonkey_style.py orange caused by a bad #include order somewhere; should be easy to fix.)
Comment 15•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #13) > (In reply to Brian Hackett (:bhackett) from comment #12) > > Yay! Can you remove JS_ION next? > > Unfortunately people are using SM on platforms for which we don't have a > JIT, and I think it'd be nice to support this. Maybe we could move more code > outside JS_ION though... That was a bit tongue in cheek but I think it would be really nice to do --- we would still be supporting --disable-ion builds, but just change out the files which are compiled rather than use an #ifdef everywhere. Filed bug 1042833.
Updated•10 years ago
|
Attachment #8460828 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/35038c3324ee https://hg.mozilla.org/integration/mozilla-inbound/rev/a0dd5a83ba36 https://hg.mozilla.org/integration/mozilla-inbound/rev/6426fef52f51 https://hg.mozilla.org/integration/mozilla-inbound/rev/e8558ecd9b16
https://hg.mozilla.org/mozilla-central/rev/35038c3324ee https://hg.mozilla.org/mozilla-central/rev/a0dd5a83ba36 https://hg.mozilla.org/mozilla-central/rev/6426fef52f51 https://hg.mozilla.org/mozilla-central/rev/e8558ecd9b16
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Updated•10 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•