Closed Bug 1266676 Opened 8 years ago Closed 8 years ago

Compile small functions faster

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: h4writer, Assigned: h4writer)

References

Details

Attachments

(4 files)

Something I noticed earlier already is that our performance increase going from cold to warm to hot code is not consequential. We have a gap between cold and hot code. We are or slow or fast, we don't improve performance incremental. This is mostly because we have a single threshold where we start compiling ion code. That threshold is because compiling takes time and we want to make sure it is beneficial to compile it. (I.e. that for short running code compiling code we don't take overhead of compiling when we won't run it long enough.

The threshold is chosen based on the average compilation cost of a normal function. As a result this gives a disadvantage to small functions. Small functions can get compiled earlier, since they will be finished quite quickly. And this will help warm, but not yet hot code.

I noticed this earlier on a benchmark till provided, but cannot find the link anymore. But this is also quite visible in the graphs arai created. Our performance difference over X iterations. In the beginning we accumulated some overhead, by running slower and as soon as we get in ionmonkey we have the same performance as before (on the regexp code). But in the meantime we have some gathered some extra execution time by first running in the baseline.
(See bug 887016 graphic "Performance comparison of match/search/replace/split with different input")

To improve the process of going from slow to fast code, we can compile smaller functions faster. The overhead is quite small and we can run those faster in ion. As a result this can improve our warm, but not hot execution code.

(This helps selfhosting quite a lot, since we introduce quite a lot of smaller functions, but also normal JS code will feel this improvement. Though not quite visible on longer running benchmarks)
Attached patch PatchSplinter Review
Compiles small functions when warmup threshold is 100. Could go further, but that increased the number of compilation considerably.

Improves current trunk from 331ms to 316.5ms on sunspider.
Assignee: nobody → hv1989
Attachment #8744236 - Flags: review?(jdemooij)
I also measured this with all patches of arai already committed. Those bring already an improvement from 331ms to 312.4ms. This extra patch improves it further from 312.4ms to 294.6ms (6%).

The improvements are mostly the regressed benchmarks from the regexp and array landing.
Comment on attachment 8744236 [details] [diff] [review]
Patch

Review of attachment 8744236 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting idea! I wonder if 100 is the right value - looking forward to AWFY results.
Attachment #8744236 - Flags: review?(jdemooij) → review+
Blocks: 1265307
Here's the performance comparison graph for same testcases as bug 887016's graph (RegExp).

Surely, it shows different characteristic in the curve between cold and hot.

In most cases:
  1) "after" curve reaches the constant gradient on Ion execution quicker than "before"
  2) "after" curve shows better performance on more than 1000 iterations, as "before" curve shows bump around 1000, but "after" curve doesn't or does smaller
  3) "after" curve shows worse performance on less than 1000 iterations, as "after" curve shows bump at around 100 iteration

maybe, some regression shown in SunSpider matches to the (3) case?  as they're all short-time testcases.
Backed out because it looks like it fails regress-476427.js.

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/3ae9a6bb0821
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=26422968&repo=mozilla-inbound

Tons of:
09:56:16     INFO -  JavaScript warning: file:///C:/slave/test/build/tests/jsreftest/tests/js1_8/extensions/regress-476427.js, line 1: JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead
09:56:17     INFO -  Hit MOZ_CRASH() at c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/src/xpcom/base/nsDebugImpl.cpp:604

Some:
09:56:17     INFO -  ### ERROR: SymInitialize: Not enough storage is available to process this command.
09:56:17     INFO -  #01: ??? (???:???)

09:56:20  WARNING -  TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/jsreftest/tests/jsreftest.html?test=js1_8/extensions/regress-476427.js | application terminated with exit code 1

Please let me know if it's caused by something different, e.g. a bad slave.
Flags: needinfo?(hv1989)
Need to look into this. Later results show this is probably not a bad slave. I assume compiling more is hitting something bad in ionmonkey.
Flags: needinfo?(hv1989)
The cause of the failure is test "regress-476427.js". It starts compiling earlier, which results in taking all the memory. So we crash due to OOM. Decreasing the total iterations on that test helps us to not OOM
Attachment #8748515 - Flags: review?(jdemooij)
Attachment #8748515 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/ecc70bad825e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Depends on: 1270108
You need to log in before you can comment on or make changes to this bug.