523497 - TM: Cut off traces with too many merges

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Since bug 516264 is unresolved, we need a way to stop explody traces from getting out of control. This currently happens in 3d-cube when we trace Loop() using recursion (tail calls, see bug 445363 comment #59).

To get an idea of what heuristics we should try, I got some statistics on TraceRecorder::cfgMerges per compile() call. Each line is the script+line where the trace ends, and the interval is the min and max value for cfgMerges.

Before tail calls:
---------------------
3d-cube.js:232 [46, 46]
3d-raytrace.js:295 [1, 11]
date-format-xparb.js:413 [0, 7]
3d-raytrace.js:191 [2, 5]
3d-raytrace.js:252 [3, 3]
access-fannkuch.js:17 [1, 3]
3d-raytrace.js:247 [2, 3]
string-base64.js:92 [1, 3]
crypto-sha1.js:61 [1, 2]
3d-raytrace.js:225 [1, 2]

After tail calls:
---------------------
3d-cube.js:232 [19, 46]
3d-raytrace.js:295 [1, 11]
date-format-xparb.js:413 [0, 7]
3d-raytrace.js:191 [2, 5]
3d-raytrace.js:252 [3, 3]
access-fannkuch.js:17 [1, 3]
3d-raytrace.js:247 [2, 3]
string-base64.js:92 [1, 3]
crypto-sha1.js:61 [1, 2]
3d-raytrace.js:225 [1, 2]

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

SunSpider results for capping cfgMerges.length() to 20:

http://pastebin.mozilla.org/677828

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://pastebin.mozilla.org/677830 with tail calls

Comment 3

[Robert Sayre]

Attachment: patch

Comment 4

[Andreas Gal :gal]

sayrer, can you please post your bugzilla password in this bug for future reference so david can post in your name without having to wait for you to come to MV and not log yourself out? ;)

Comment 5

[Robert Sayre]

(In reply to comment #3)
> Created an attachment (id=407440) [details]
> patch

nice patch, sayrer

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 407440 [details] [diff] [review]
patch

This didn't seem to have adverse effects on MozillaTry.

Comment 7

[Nicholas Nethercote [inactive]]

Cachegrind gives these instruction counts:

3d-cube:
  total         125,130,318     116,534,135     1.074x better
  on-trace       25,345,708      25,145,417     1.008x better

Nothing else changed much.  SS timings indicate a 9% (~3.5ms) speedup for 3d-cube, again with nothing else changed.  V8 changes all appear to be within noise.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: rebased patch

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: rebased again (against TM 53109:7f7c60f205f6)

I just measured SS again with TM+JM.  This patch has zero effect.  The only one it was helping before was 3d-cube, and that was due to tracing of recursion, something which no longer happens.

Comment 10

[Paul Wright]

So is this to be marked RESOLVED INVALID?