Closed Bug 837014 Opened 12 years ago Closed 12 years ago

IonMonkey: Polymorphic inline for deltablue Plan.execute()

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: sstangl, Unassigned)

References

Details

Attachments

(1 file)

]Deltablue is divided into independent halves, chainTest() and projectionTest(), each of which is roughly a SunSpider-style benchmark. We are about equal with v8 on projectionTest() (52000 versus 53000), but slower on chainTest() (40000 versus 55000). On the chainTest() half, we miss an important polymorphic inlining opportunity. This is the main loop of chainTest(): > Plan.prototype.execute = function () { > for (var i = 0; i < this.size(); i++) { > var c = this.constraintAt(i); > c.execute(); > } > } The constraint objects in play are one of StayConstraint, EditConstraint, and EqualityConstraint. Their .execute() functions are defined as follows: > StayConstraint.prototype.execute = function () { > // Stay constraints do nothing > } > EditConstraint.prototype.execute = function () { > // Edit constraints do nothing > } > EqualityConstraint.prototype.execute = function () { > this.output().value = this.input().value; > } The benchmark sets up these objects into a chain such that there are N EqualityConstraints but only 1 StayConstraint at the end and a few interspersed EditConstraints. Because of this, when we go to inline, we find that the three given targets have the following useCounts: > StayConstraint: 1 > EditConstraint: 122 (saved by js_IonOptions.inlineUseCountRatio) > EqualityConstraint: 10187 And so the Oracle vetos the inlining due to the one low useCount, and the main loop winds up using visitCallGeneric(). As you are noticing, this is very silly, because that function doesn't do anything. There are a few options to solve this in a more generic way, but the easiest solution is to just check the script length and ignore the useCount entirely if the script is trivial (length 1). There's also an MGetPropertyCache that shouldn't be there after inlining, for follow-up work.
Attached patch Always inline the empty script. (deleted) — Splinter Review
chaintest(): 40000 -> 56000 (v8 55000) deltablue: ~20150 -> ~20500 (v8 23571) The benchmark is divided into halves: we are now faster than v8 on the former and equally fast on the latter, but slower on the whole. Possibly some bad information is bleeding through the barrier between them.
Attachment #708907 - Flags: review?(bhackett1024)
Attachment #708907 - Flags: review?(bhackett1024) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 843866
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: