Closed Bug 648321 Opened 12 years ago Closed 11 years ago

Continue using inline slots after slot array is allocated


(Core :: JavaScript Engine, defect)

Not set





(Reporter: billm, Assigned: bhackett1024)




(2 files, 1 obsolete file)

In bug 584917, we decided to stop using inline slots at all after allocating a slot array. This simplifies the code, and it also makes sense for things like dense arrays.

However, in cases like the kraken ai-astar benchmark, we would save memory and improve performance by continuing to use the inline slots data for low-numbered slots. The ai-astar benchmark constructs a ton of objects with 5 properties. After they've all been constructed it adds 4 more properties to all of them. Since most later accesses are to one of the first 5 properties, it would be nice if we could continue to use the inline slot here. Also, it would save memory.
Assignee: general → bhackett1024
Attached patch patch (obsolete) — Splinter Review
Make the changes described above.  Kraken improves by 3% (ai-astar by 35%), v8bench improves by 1%, SS regresses by 4%.  I need to figure out what's going on with the regression, the changes here are pure improvement for JIT code but VM slot accesses can slow down significantly.  Most of the SS regression looks to be in tests that probably spend a lot of time in the VM (tofte, tagcloud), will look to see if we need to either mitigate or eliminate the VM overhead.
Attached patch patchSplinter Review
This wipes out the perf loss on SS and keeps the gain on kraken and v8bench by making numFixedSlots much faster.  The number of fixed slots is stashed in the top 5 bits of the object's flags (which have plenty of free space), so we can get them with a load and shift rather than several dereferences to get the arena and a switch statement.  This is also more forward-compatible with a generational GC, as we don't depend on the current arena to figure out numFixedSlots.
Attachment #524716 - Attachment is obsolete: true
Attachment #524772 - Flags: review?(luke)
Comment on attachment 524772 [details] [diff] [review]

I'm not very knowledgeable in this area.  Perhaps Jason?
Attachment #524772 - Flags: review?(luke)
Attachment #524772 - Flags: review?(jorendorff)
Patch landed on JM.  This increases the divergence between the JM and TM branches, but unblocks forthcoming property-access optimizations and hopefully this bug will have landed on TM by the time we get interest in merging back from JM.
Depends on: 650912
Depends on: 649152
Review ping.
Comment on attachment 524772 [details] [diff] [review]

Canceling review, folding this into bug 657412.
Attachment #524772 - Flags: review?(jorendorff)
Closed: 11 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.