Closed Bug 942105 Opened 6 years ago Closed 6 years ago

IonMonkey: Remove bogus inlineUseCountRatio

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: h4writer, Assigned: h4writer)

References

Details

(Whiteboard: [qa-])

Attachments

(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

Octane2.0:
+ 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]
bug942105-remove-inline-use-count-ratio

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 ;)

https://hg.mozilla.org/integration/mozilla-inbound/rev/78f9a7685da3
Backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/67cea1cdd626 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, https://tbpl.mozilla.org/php/getParsedLog.php?id=31023182&tree=Mozilla-Inbound 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:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39bfcadd6492
https://hg.mozilla.org/mozilla-central/rev/39bfcadd6492
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 946243
No longer depends on: 946243
Whiteboard: [qa-]
Depends on: 981325
Depends on: 995564
Depends on: 995675
No longer depends on: 995564
Depends on: 1076283
You need to log in before you can comment on or make changes to this bug.