JSCodeGenerator::upvarMap should use exactly as much space as it needs

RESOLVED FIXED in mozilla8



JavaScript Engine
6 years ago
6 years ago


(Reporter: cdleary, Assigned: cdleary)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

Created attachment 545268 [details] [diff] [review]
Allocate less for upvarMap.

appendN was causing us to over-allocate. I also took a look at the number of upvar cookies we typically allocated on parsemark and they have low averages, so I added some inline storage.

Attachment #545268 - Flags: review?(nnethercote)
Comment on attachment 545268 [details] [diff] [review]
Allocate less for upvarMap.

Review of attachment 545268 [details] [diff] [review]:

Looks plausible.  But I really want to see the memory regression disappear!  Can you report back to confirm that it's happened?  If it doesn't fix the regression, I think we should back both this and bug 669786 out.
Attachment #545268 - Flags: review?(nnethercote) → review+
Whiteboard: [inbound]
Backed out of m-i as part of a mass backout.
Whiteboard: [inbound]
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
It looks like the regression was fixed:


Am I reading that right?
(In reply to comment #4)
> Am I reading that right?

I never landed the followup patch on tracemonkey, so I'm skeptical.
If we don't get clear evidence that this fixed the original regression by early next week, I'm going to back out this patch and the previous one.  I don't want to accept a regression due to time passing and everyone forgetting about it because the bug was marked FIXED.
(In reply to comment #6)

We never saw a regression report on mozilla-central, so the two patch combo didn't regress anything. TM was frozen after the first one landed and before the two-patch combo went to inbound.
(In reply to comment #7)
> (In reply to comment #6)
> We never saw a regression report on mozilla-central, so the two patch combo
> didn't regress anything.

So the two patches landed separately on mozilla-central, and we never saw a regression?  That makes it sound like the TM regression was bogus, though it's odd that it seemingly lasted for a few pushes before going away.  Huh.  Ok, I'm satisfied.
(In reply to comment #8)
> Ok, I'm satisfied.

Yay! We potentially malloc less stuff now because of the inline vector size for typical upvarmaps, so that makes me happy.
You need to log in before you can comment on or make changes to this bug.