Closed
Bug 669786
Opened 10 years ago
Closed 10 years ago
JSCodeGenerator::upvarMap should be a js::Vector
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: cdleary, Assigned: cdleary)
References
Details
(Whiteboard: [fixed-in-tracemonkey])
Attachments
(1 file)
7.04 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
Spotted this when I was working on another bug.
Attachment #544371 -
Flags: review?(nnethercote)
![]() |
||
Comment 1•10 years ago
|
||
Comment on attachment 544371 [details] [diff] [review] upvarMap as vector. Review of attachment 544371 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine, but the point is unclear. I guess it's good to replace an ad-hoc array type with js::Vector? But there's still several other JSUpvarArrays around; getting rid of all of them would be a bigger win. ::: js/src/jsemit.cpp @@ +2341,5 @@ > + size_t lexdepCount = cg->roLexdeps->count(); > + > + JS_ASSERT_IF(!upvarMap.empty(), lexdepCount == upvarMap.length()); > + if (upvarMap.empty()) > + upvarMap.appendN(UpvarCookie(), lexdepCount); Probably better to use infallibleAppendN(), it makes the precondition clearer.
Attachment #544371 -
Flags: review?(nnethercote) → review+
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to comment #1) > This seems fine, but the point is unclear. I guess it's good to replace an > ad-hoc array type with js::Vector? But there's still several other > JSUpvarArrays around; getting rid of all of them would be a bigger win. The other ones are uses of the type embedded in JSScript, and we hate increasing the size of JSScript for memory footprint reasons. Second comment is good -- it's actually fallible and I forgot to check! It's just a lazy vector capacity initialization like in the original.
Assignee | ||
Comment 3•10 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/1013f4be025f
Whiteboard: fixed-in-tracemonkey
Assignee | ||
Updated•10 years ago
|
Whiteboard: fixed-in-tracemonkey → [fixed-in-tracemonkey] [inbound]
Backed out of m-i as part of a mass backout.
Whiteboard: [fixed-in-tracemonkey] [inbound] → [fixed-in-tracemonkey]
Comment 5•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2bdcc80c8324
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in
before you can comment on or make changes to this bug.
Description
•