Closed
Bug 390175
Opened 17 years ago
Closed 17 years ago
ActionMonkey: Add CustomMark() methods to SpiderMonkey types
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(1 file, 1 obsolete file)
7.40 KB,
patch
|
Mardak
:
review+
|
Details | Diff | Splinter Review |
This is a pretty simple change. Unfortunately I see no good way of testing this until we make the switch to MMgc allocation.
Attachment #274488 -
Flags: review?(edilee)
Assignee | ||
Comment 1•17 years ago
|
||
Btw- this patch installs on top of the one in bug 389420.
Updated•17 years ago
|
Attachment #274488 -
Attachment is patch: true
Attachment #274488 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 2•17 years ago
|
||
Note: The CustomMark() methods in this patch don't actually call MMgc::GC::SetMark() yet, because we're not using MMgc for allocation yet -- it would crash. But when we switch to MMgc, the line in JS_CallTracer that reads: *flagp |= GCF_MARK; will change to use SetMark(), so these objects will comply with what MMgc requires.
Comment 3•17 years ago
|
||
Comment on attachment 274488 [details] [diff] [review] v1 >+protected: >+ /* Helper function for implementing MMgc::GCFinalizedObject::Mark(). */ >+ void MarkAs(uint32 kind); Should be "for implementing JSFinalizedGCThing::CustomMark. This could return bool to simplify switching things to use MMgc later.. see below. >+bool >+JSObject::CustomMark() >+{ >+ MarkAs(JSTRACE_OBJECT); >+ return true; > } We're returning true here even though we don't MMgc::GC::SetMark(). We could |return MarkAs()| and have MarkAs return false for now until we replace the GCF_MARK.
Assignee | ||
Comment 4•17 years ago
|
||
I think the comment should say "MMgc::GCFinalizedObject::CustomMark()", because that's where the method is actually declared and documented. (There is no JSFinalizedGCThing::CustomMark.)
>We're returning true here even though we don't MMgc::GC::SetMark().
As soon as we switch to MMgc for allocation, these methods will honor the full CustomMark() contract to the letter.
Right now, they honor the *spirit* of the contract--they each return true, indicating the object and everything reachable from it is marked. Whether it's GC::SetMark()ed or GCF_MARKed is not the right way to look at it, I claim.
Comment 5•17 years ago
|
||
Comment on attachment 274488 [details] [diff] [review] v1 (In reply to comment #4) > I think the comment should say "MMgc::GCFinalizedObject::CustomMark()", because > that's where the method is actually declared and documented. Even better. > >We're returning true here even though we don't MMgc::GC::SetMark(). > Right now, they honor the *spirit* of the contract Fair enough.
Attachment #274488 -
Flags: review?(edilee) → review+
Assignee | ||
Comment 6•17 years ago
|
||
Shake off bitrot; add one other trivial cleanup (say "ScanDelayedChildren(trc)" instead of "ScanDelayedChildren(rt->gcMarkingTracer)").
Attachment #274488 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Attachment #275168 -
Flags: review?(edilee)
Comment 7•17 years ago
|
||
Comment on attachment 275168 [details] [diff] [review] v2 >+ * Helper function for implementing >+ * MMgc::GCFinalizedObject::CustomMark(). >+ */ >+ void MarkAs(uint32 kind); r+ with ()s removed from comment.
Attachment #275168 -
Flags: review?(edilee) → review+
Assignee | ||
Comment 8•17 years ago
|
||
changeset 3743: 08fddac94d35
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•