An issue that is making tuning hard is that when we exit or blacklist a trace, we can end up having to run in the interpreter for a fairly long time before we reach a safe point. When this happens, perf gets very bad. So, when we try to tune by not tracing some loop that doesn't trace well, we can kill system perf. This is very bad.

This patch, written by dvander, fixes the basic problem, but it causes a SunSpider regression of 6 ms or so. This is mostly on 3d-raytrace. So, to finish this bug, we need an additional patch that solves that problem. I don't know the exact cause of the regression yet, but dvander says it's basically that this patch allows more to trace than before, and 3d-raytrace runs better in JM, so that doesn't help.

I found that setting MIN_LOOP_ITERS to 16, suggested by billm, mostly fixes the 3d-raytrace problem: then we take a regression of only 2 ms there. And the patch does win in various places, so that would take care of things, *except* that we then get a 3-4ms regression on spectral-norm, so we still take a 3ms regression overall. And I really hope this bug can actually make us faster overall, so there is more to do.

(In reply to comment #1) > > I don't know the exact cause of the regression yet, but dvander says it's > basically that this patch allows more to trace than before, and 3d-raytrace > runs better in JM, so that doesn't help. A big reason why 3d-raytrace sucks when traced is because we no longer force numeric array elements to be got as doubles. Doing so helped fannkuch but hurt 3d-raytrace and 3d-cube (I think). Bring on type inference.

(In reply to comment #2) > I found that setting MIN_LOOP_ITERS to 16, suggested by billm, mostly fixes the > 3d-raytrace problem: then we take a regression of only 2 ms there. And the > patch does win in various places, so that would take care of things, > > *except* that we then get a 3-4ms regression on spectral-norm, so we still take > a 3ms regression overall. And I really hope this bug can actually make us > faster overall, so there is more to do. Just to add a little more: setting MIN_LOOP_ITERS to 8 instead of 16 fixes the spectral-norm regression without compromising anything else, so we end up with a small win. We might be able to get a better constant by multiplying the number of iterations by the length of the trace and blacklisting based on that. 3d-raytrace has a reasonably small loop of 3 iterations at line 235 that we need to avoid tracing, while spectral-norm has a big loop of 10 iterations at line 49 that we want to trace.

This is the same patch, with Bill's suggested parameter fixes. I get a 1.5% win on SS, a big win (80 points, ~20%) on Dromaeo, and a possible 0.5% regression on V8. I would like to take it as it is, because Bill is going to keep tuning anyway. The point is to get the improvements to the tracing integration into TM so that tuning becomes easier (and the patch doesn't rot).

Comment on attachment 472862 [details] [diff] [review] Patch with tweaked tuning parameters OK. This patch was originally written by dvander. So I reviewed it, and it makes sense to me. But before he lands it, he should check on my changes, which were: - rebase for syntactic changes - rebase for igor's build fix. In particular, I had to change stuff near the definition of CLEAR_LEAVE_ON_SAFE_POINT, so check that carefully. - change HOTLOOP to 8 and MIN_ITERS to 8 per Bill's suggestion.

Comment on attachment 472862 [details] [diff] [review] Patch with tweaked tuning parameters Nice, thanks Dave & Bill for the help.

Backed out as 7219df6c126c 10615 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_popupincontent.xul | shorter menu again height shortened - got 271, expected 76 On what seems like every platform.

While debugging this failure, I found this code to be related and unnecessary. JaegerShot was copying the current outgoing JSFrameRegs into the old regs. The only reason this could be needed is for generators (which want to know the exit stack & pc), but we don't compile those and they deserve a different solution when the time comes.

And the actual fix. RemoveExcessFrames() was using a stale |fp|, rather than reloading from |cx->fp()|. So the return value did not propagate correctly.