ObjectGroup's pretenuring flag is not always respected

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
3 months ago
a month ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

(Blocks 1 bug)

Trunk
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 wontfix, firefox68 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 months ago

It seems there are several cases when we don't pretenure objects even though the should pretenure flag is set on the group.

(Assignee)

Comment 1

2 months ago

This adds an overload of GetInitialHeap that takes an ObjectGroup* instead of a Class* and also takes into account whether the group's shouldPreTenure flag is set. I moved this to JSObject-inl.h too.

I removed the heap parameter in a few places, in particular in NewDenseCopyOnWriteArray which required a bunch of changes elsewhere including the JITs. I left the heap parameter intact for environment objects where we may have reason prefer these objects to be allocated in the tenure heap. It's possible we should just remove all these parameters too and make allocation more uniform.

Comment 3

2 months ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b4fd78107e2
Fix places where we don't respect the shouldPretenure flag when creating an object r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/82a1c7087806
Assert that the group's shouldPretenure flag is respected when creating an object r=jandem

Comment 4

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment 5

2 months ago
Backout by aiakab@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/e6a4cd115b45
Backed out 2 changesets on request of pascalc for causing Bug 1534118 a=backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Can you guys please take a look at this?

Flags: needinfo?(pascalc)
Flags: needinfo?(jcoppeard)

Comment 7

2 months ago
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/2369f5bbf379
Fix places where we don't respect the shouldPretenure flag when creating an object r=jandem
https://hg.mozilla.org/mozilla-central/rev/2bec5d831e88
Assert that the group's shouldPretenure flag is respected when creating an object r=jandem

This was backed out by mistake and it was relanded now.

Status: REOPENED → RESOLVED
Last Resolved: 2 months ago2 months ago
Flags: needinfo?(pascalc)
Flags: needinfo?(jcoppeard)
Resolution: --- → FIXED

Comment 9

2 months ago
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/f4c23517cec8
Backed out 2 changesets for causing Bug 1534118 a=backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1534118
Depends on: 1533873
Target Milestone: mozilla67 → ---
Depends on: 1533927

Hi Jon, Bug 1534118 comment 0 has a test case that crashes with this change.

Flags: needinfo?(jcoppeard)
(Assignee)

Comment 12

2 months ago

It turns out that the JITs assume certain types of environment objects are always allocated in the nursery and that they can omit postbarriers for such objects. This was causing the crash in comment 11.

Flags: needinfo?(jcoppeard)

Comment 13

2 months ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6af6ae0901cc
Fix places where we don't respect the shouldPretenure flag when creating an object r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/49304ded1ad6
Assert that the group's shouldPretenure flag is respected when creating an object r=jandem

Comment 14

2 months ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 2 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Comment 16

2 months ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c79959d9237d
Fix places where we don't respect the shouldPretenure flag when creating an object r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ed792328e9f
Assert that the group's shouldPretenure flag is respected when creating an object r=jandem
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.