Closed
Bug 652931
Opened 14 years ago
Closed 14 years ago
Proper handling of large mark stack insertion
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gwagner, Assigned: gwagner)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
I noticed that for the FOTN the large markstack gets pretty full.
Assignee | ||
Updated•14 years ago
|
Attachment #528404 -
Flags: review?(wmccloskey)
Wait, this seems like a serious bug to me. Is there any guarantee that the assertion will succeed? If push returns false, we should probably just do the normal thing and not use the large object path. Thanks for finding this!
Assignee | ||
Comment 3•14 years ago
|
||
Yeah I wanted to ask you why we don't check the return value there. I will come up with another patch.
Assignee | ||
Updated•14 years ago
|
Summary: Add success assertion for large mark stack insertion → Proper handling of large mark stack insertion
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #528404 -
Attachment is obsolete: true
Attachment #528404 -
Flags: review?(wmccloskey)
Assignee | ||
Updated•14 years ago
|
Attachment #528863 -
Flags: review?(wmccloskey)
Comment on attachment 528863 [details] [diff] [review] patch Thanks. Another way to do it is like this: if (...is large dense array... && gcmarker->largeStack.push(LargeMarkItem(obj))) /* do thing, push succeeded */; else clasp->trace(gcmarker, obj); I'm not sure if this is better or worse, but it avoids code duplication.
Attachment #528863 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 6•14 years ago
|
||
Yeah I was thinking of the same code but I have never seen it in SM so I didn't want to start with it :)
Assignee | ||
Comment 7•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/ebc5f8a4b233
Whiteboard: fixed-in-tracemonkey
Comment 8•14 years ago
|
||
cdleary-bot mozilla-central merge info: http://hg.mozilla.org/mozilla-central/rev/ebc5f8a4b233
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9) > Do we need this fix on Aurora or Beta? Depends if bug 616666 is in there.
(In reply to comment #12) > Bill is the explicit mark stack on Aurora? It seems like Aurora branched on 4/12 and bug 616666 went in on 4/26. So we're good. It would be nice if there were a web page that listed the time and changeset id of each branch. Maybe there's an easy way to figure this out, but I ended up just searching through hg.mozilla.org, which is pretty annoying.
Comment 14•13 years ago
|
||
> It would be nice if there were a web page that listed the time and changeset
> id of each branch. Maybe there's an easy way to figure this out, but I ended
> up just searching through hg.mozilla.org, which is pretty annoying.
Just let cdleary-bot set the target milestone in bugzilla?
You need to log in
before you can comment on or make changes to this bug.
Description
•