Closed
Bug 328896
Opened 18 years ago
Closed 18 years ago
MarkGCThing: stricter tail recursion
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: igor, Assigned: igor)
References
Details
Attachments
(2 files, 1 obsolete file)
5.83 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
5.83 KB,
patch
|
Details | Diff | Splinter Review |
Currently MarkGCThing from jsgc.c contains the following code: start: ... if (*flagp & GCF_MARK) { /* * This should happen only if recursive MARK_GC_THING marks flags * already stored in the caller's *next_flagp. */ goto out; } The situation described in the comments happens during marking of object's slots when recursive MARK_GC_THING invocation marks object referenced through next_thing. It is better to move the check there and add assert to require *flagp to be unmarked.
Assignee | ||
Comment 1•18 years ago
|
||
Besides moving the code to check for a marked thing to where it belongs the patch changes the return type for NarkGCThing to void and makes tail recusrion eleimination slightly more readable. See also bug 324278.
Comment 2•18 years ago
|
||
Comment on attachment 213506 [details] [diff] [review] Implementation and clenups Cool -- only thoughts are these very minor comments: 1. Save one goto by putting onTailRecursion: label and code in place of the patch's second goto onTailRecursion. 2. SpiderMonkey has an esoteric style rule that prefers out_of_memory for long label names, not camelCaps. You are now entering the ascended master SpiderMonkey level ;-). /be
Attachment #213506 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 3•18 years ago
|
||
Besides addressing the nits the patch removes "out" label replacing all "goto out;" by "break;".
Attachment #213506 -
Attachment is obsolete: true
Attachment #214509 -
Flags: review?(brendan)
Comment 4•18 years ago
|
||
Comment on attachment 214509 [details] [diff] [review] Nits etc. Great! Thanks, /be
Attachment #214509 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 5•18 years ago
|
||
Assignee | ||
Comment 6•18 years ago
|
||
I committed the last patch: Checking in jsgc.c; /cvsroot/mozilla/js/src/jsgc.c,v <-- jsgc.c new revision: 3.119; previous revision: 3.118 done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•