The default bug view has changed. See this FAQ.

remove shell workers

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Benjamin, Assigned: Benjamin)

Tracking

unspecified
mozilla16
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 639439 [details] [diff] [review]
remove them
Attachment #639439 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #639439 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #639439 - Flags: review?(jorendorff)
Attachment #639439 - Flags: review?(jorendorff) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed

Comment 2

5 years ago
(Phew.)
https://hg.mozilla.org/integration/mozilla-inbound/rev/d64dfd57d873
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla16
Assignee: general → bpeterson
https://hg.mozilla.org/mozilla-central/rev/d64dfd57d873
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Were shell workers preventing other stuff from going forward, or is this just to get rid of some unimportant code?  Why were they added in the first place?  (I'm not criticizing the removal, I'm just curious.)

Comment 6

5 years ago
If anybody could comment on the need for removal, that would be awesome.  I want to run web-workers-like code in a non-browser embedding, and keep it working post JS 1.8.7 -- knowing where the landmines are ahead of time would be quite helpful.
(In reply to Wesley W. Garland from comment #6)
> If anybody could comment on the need for removal, that would be awesome.  I
> want to run web-workers-like code in a non-browser embedding, and keep it
> working post JS 1.8.7 -- knowing where the landmines are ahead of time would
> be quite helpful.

I have no idea why they were added.  I do know that they were under js/src/shell/ and thus only in the js shell and not available to embeddings.  It's for the best, really: the code was in an obviously bit-rotted state.  I'd have agitated for either removing or rewriting it if it were actually available to embeddings.  As is, I just ignored it and hoped it would go away :-).
Sorry I didn't respond here sooner.

JS shell workers were added just-pre-compartments so we could have unit tests for the multithreaded JSAPI patterns we knew we would have to continue to support.

There was nothing horribly *wrong* with that code, and certainly the general approach to multithreading is still basically what embedders should do. This was just deleted because the maintenance costs were senseless, given we have even better tests for (real) Web workers.

The API is evolving faster than ever. It was a legitimate pain in the butt to keep updating shell workers for every little thing. (Especially GC work. The data structures in the shell worker code were nontrivial; switching to handles was going to be a big pain.)
Sorry, I did not mean to imply that the code was wrong, only a bit moldy. :-)
You need to log in before you can comment on or make changes to this bug.