Closed Bug 1241311 Opened 4 years ago Closed 4 years ago

Pre-tenure SavedFrame objects


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox46 --- fixed


(Reporter: fitzgen, Assigned: fitzgen)


(Blocks 1 open bug)



(1 file)

Because of SavedStacks' tail-sharing, we cut down on the number of SavedFrame
allocations, but they also tend to live fairly long. When using the devtools'
profiler on Octane, I noticed that we were spending much more time in GC when
recording allocation stacks (as SavedFrame stacks) than when we were not
recording allocation stacks. We were spending about 30% of time in nursery
collections and 7% of time in major GCs. This commit makes it so that SavedFrame
objects are always allocated in the tenured heap. After this change, only about
17% of time is spent in nursery collections and 8% in major GCs.
Assignee: nobody → nfitzgerald
Blocks: 1205782
Try push:

Note that it is somewhat expected that we spend more time in GC when recording allocation stacks because (a) the allocation stacks are GC things and so they put pressure on the GC and (b) we're running a bunch of devtools JS code along with the mutator/profilee. However, we should strive to keep the additional overhead as low as practical.
Comment on attachment 8710168 [details] [diff] [review]
Pre-tenure SavedFrame objects

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

Good find! Any clue why our pre-tenuring optimization was not hitting here?
Attachment #8710168 - Flags: review?(terrence) → review+
I dug in a little bit and couldn't figure out why the pre-tenuring optimization was not hitting. If I was on linux right now, I could rr and probably find out for sure. But since I'm not on linux right now and am only exploring code paths via dxr, my best guess at the moment is that the Object.freeze'ing we do to every SavedFrame object and the SavedFrame.prototype messes with the shape and/or object group which eventually results in OBJECT_FLAG_UNKNOWN_PROPERTIES being set which disables pre-tenuring. This is wild speculation, and someone who was actually familiar with TI could probably make a much more educated guess.
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.