Closed
Bug 394297
Opened 17 years ago
Closed 16 years ago
Make MMgc thread-safe (as a compile-time option)
Categories
(Tamarin Graveyard :: Garbage Collection (mmGC), defect)
Tamarin Graveyard
Garbage Collection (mmGC)
Tracking
(Not tracked)
VERIFIED
WONTFIX
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(2 obsolete files)
This was briefly discussed on the tamarin-devel mailing list: https://mail.mozilla.org/pipermail/tamarin-devel/2007-August/000013.html There are several pieces to this. Here is one possible plan (not necessarily *the* plan): - Lift the SpiderMonkey request model into MMgc. http://developer.mozilla.org/en/docs/SpiderMonkey_Internals:_Thread_Safety Just as in SpiderMonkey, this would be #ifdef'd out by default, enabled at compile time with -DMMGC_THREADSAFE or such. - Ensure that GC::Collect scans not only the calling thread's stack, but all threads that are in requests. (This is pretty easy to implement: when a thread calls SuspendRequest, just root its stack.) - Make DRC thread-safe. (It's not clear to me that anyone wants this. The story could be "RCObject is never thread-safe -- always use GCObject for objects that may be seen by multiple threads".) - Make incremental GC thread-safe. https://mail.mozilla.org/pipermail/tamarin-devel/2007-August/000017.html
Assignee | ||
Comment 1•17 years ago
|
||
This is now an ActionMonkey Stage 1 goal. I'm doing this now mainly to support efforts currently underway (by bsmedberg and taras) to convert Mozilla's XPCOM objects from AddRef/Release to MMgc. I think the above is the approach we want to take. Making DRC at all safe for MT code is not included in this bug for now.
Assignee | ||
Comment 2•17 years ago
|
||
Phew. I thought this would be done yesterday, or even Wednesday, but I got more confused than I thought I would! I wanted to make a patch that is simple and contains the essence of the request model. It would have been easier to submit 2000+ lines of incomprehensible spaghetti code. Here's a snapshot of the simplified version. Untested, but the concepts are there. There are <500 lines of new code. There are very few changes to MMgc in it's default non-MMGC_THREADSAFE configuration, and I think all of those are trivial. Flip on MMGC_THREADSAFE, and you get the "multiple application threads or one GC thread" behavior, and each GC scans all threads' stacks. But this lacks several features SpiderMonkey will need: sentry objects to enter and leave requests; request contexts; multiple (nested) requests per "context"; multiple contexts per thread; public locking methods for GC::m_lock; and restarting GC as needed when multiple threads ask for GC at around the same time. I think all these are pretty easy to add.
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•17 years ago
|
||
This patch is for people to skim, not to apply. It's got "infinite" diff-context, so the Bugzilla "Diff" link should show the whole file. Take a look; focus on GC.h, where I've added a ton of comments. Those funny-looking @access comments document the extra access restrictions on methods and fields in MMGC_THREADSAFE builds. Documenting these restrictions is (imho) very important; using this shorthand is a lot more concise than using English. For an explanation of how to read them, read the comment on MMgc::GC::m_lock (in the patch), or the first section of http://wiki.mozilla.org/MMgc_thread_safety_annotations .
Attachment #284701 -
Attachment is obsolete: true
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 286326 [details] [diff] [review] v0a - snapshot A newer version of the patch is in bug 395962.
Attachment #286326 -
Attachment is obsolete: true
Comment 5•16 years ago
|
||
Isn't this done now? Bug 395962 is now RESO/FIX.
Assignee | ||
Comment 6•16 years ago
|
||
Bug 395963 is still open. Coincidentally that's what I'm working on today.
Assignee | ||
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → WONTFIX
Updated•15 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•