Closed Bug 942105 Opened 10 years ago Closed 10 years ago

IonMonkey: Remove bogus inlineUseCountRatio


(Core :: JavaScript Engine: JIT, defect)

Not set





(Reporter: h4writer, Assigned: h4writer)



(Whiteboard: [qa-])


(1 file)

In bug 941028, I already explained inlineUseCountRatio is bogus and that we should remove it. With bug 941028 landed, we can remove it.

Just to reiterate the reason:
- If compilation cost would cost nothing, usecount should never get higher than 1000. As a result the test would always be true.
- Ratio is tested on usecount. But usecount higher than 1000 are strange, but happen if we bailout. As a result we can decide to inline a function first and after invalidation don't inline anymore. Because caller has run more in baseline than callee. (Even though both are called the same number of times).

So I removed the check. I also removed the useCount check for the caller, since it is now obsolete. The caller always has an usecount > 125. Since it only gets compiled when usecount == 1000, and all callees only get inlined when usecount == 125.

kraken: 1147.3ms to 1145.0ms
+ ai: 84.9ms to 78.8ms
+ audio: 344.5ms to 342.6ms
+ imaging: 294.6ms to 291.7ms
- json: 82.9ms to 84.5ms
- stanford: 340.4ms to 344.4ms

SS: no change

+ Richards: 28058 to 30120
+ Deltablue: 36735 to 37224
Assignee: nobody → hv1989
Attachment #8336730 - Flags: review?(jdemooij)
Depends on: 941028
Summary: IonMonkey: Remove bogus → IonMonkey: Remove bogus inlineUseCountRatio
Comment on attachment 8336730 [details] [diff] [review]

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

Always good to have simpler heuristics, and nice benchmark wins! Bug 777583 added this for sunspider 3d-raytrace though, please double check we don't regress that test. r=me with that.
Attachment #8336730 - Flags: review?(jdemooij) → review+
Tested SS again, but again saw no differences. So ss-3d-raytrace should be fine with this ;)
Backed out in for what might be the stupidest reason for a backout I've ever had. You made jit-test/tests/asm.js/testZOOB.js | --ion-eager * time out, only on debug ASan, and the next two in a row.

Now, we don't care about make check on debug ASan, bug 941084 is going to disable it, but it hasn't yet done so, leaving me stuck with permaorange unless I back out something we want, in order to keep green something we don't want.
We are possibly inlining more in testZOOB now. As a result we get bigger graphs and our debug builds checks very thoroughly if the graph is good, but that doesn't scale well. So I've disabled that for testZOOB.js. (Just like we do for testBullet.js)

Try was green. So trying again:
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
No longer depends on: 946243
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.