1644243 - Specify heap (nursery/tenured) when allocating strings

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Steve Fink [:sfink] [:s:]]

For a test in bug 1568923, I wanted to be able to allocate a tenured string, even when nursery strings are enabled. This required plumbing a heap parameter through lots of places. I took the opportunity to examine each allocation site and decide whether it might make more sense to pretenure.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1644243 - Allocate strings in the tenured heap if they will be atomized right after, and pass through InitialHeap to string allocation

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1644243 - Allocate strings in the tenured heap if they will be atomized right after

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1644243 - Default the initial heap

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1644243 - Pass InitialHeap to *all* string allocations

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1644243 - Add a newString testing function for creating various types of JSString

Comment 6

[Jon Coppeard (:jonco)]

This is a bigger change than I realised. But I guess we will need this if we want to pretenure strings by site anyway right?

Comment 7

[Steve Fink [:sfink] [:s:]]

(In reply to Jon Coppeard (:jonco) from comment #6)

This is a bigger change than I realised. But I guess we will need this if we want to pretenure strings by site anyway right?

Yeah, this is why I did the weird delayed pretenuring thing initially.

I'm not entirely sure how much of this we would need for site-specific pretenuring. There might only be a few relevant C++ allocation sites, but I don't know. I think this is still worth doing because it revealed a number of places where we should already be pretenuring, and in fact I purely slowed things down when allocating into the nursery (for atoms). Also, it didn't seem like there was any clean way to only do a subset of the callsites.

I didn't expose heap control to the external API. I haven't even looked to see whether that would be useful.

The one place I'm unsure of is where I pass in a constant gc::DefaultHeap value to the JIT. I could create a separate C++ function for doing that and avoid a small amount of JIT code bloat.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bffd2444c17
Allocate strings in the tenured heap if they will be atomized right after, and pass through InitialHeap to string allocation r=jonco
https://hg.mozilla.org/integration/autoland/rev/ccb2b543e09c
Add a newString testing function for creating various types of JSString r=jonco

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9bffd2444c17
https://hg.mozilla.org/mozilla-central/rev/ccb2b543e09c