481419 - trace repeated execution of scripts

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Mike Shaver (:shaver emeritus)]

Attachment: treat the entry pc for a script as a loop header

With the attached patch, we trace evals-within-loop as well as repeated script execution as from setTimeout and event handlers.  We also regress unpack-code performance by 10x, so there's something not quite right!

Comment 1

[Brian Crowder]

Has anyone got thoughts on the regression shaver mentions in comment 0?

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Has anyone got thoughts on the regression shaver mentions in comment 0?

Someone get jit stats with and without the patch, post them here.

/be

Comment 3

[Andreas Gal :gal]

The patch needs a threshold counter per script.

Here is what we do for loops:

- the emitter emits a JSOP_LOOP in every loop header
- at the backedge if jump target == JSOP_LOOP then we look up the fragment using target pc as key in the fragment cache, and then increment the counter in the fragment or trigger compiled code
- if we ever decide to blacklist the location, we change LOOP to NOP. both are single byte opcodes so concurrency is not an issue

What this patch needs:

- rename JSOP_LOOP to JSOP_TRACE
- change emitter to emit TRACE at every function entry, and use its PC as key (it will never be a loop target, if thats the case there is a TRACE under it)
- use some reasonable threshold and the mechanics in the patch to trigger tracing

Comment 4

[Brian Crowder]

Attachment: refreshed

Attachment 365435 [details] [diff] is badly rotted, here's a swat at refreshing it.  It crashes on my test script.

Comment 5

[Brian Crowder]

Attachment: crashes my patch

Comment 6

[Brian Crowder]

Attachment: with less crashiness

Comment 7

[Brian Crowder]

Attachment: even less crashy!

This one actually runs sunspider, even.  Test results shortly.

Comment 8

[Brian Crowder]

Attachment: sunspider results

unpack-code is no longer 10x slow (just a terrible 2x slower).  Nothing else is improved measurably, except perhaps access/binary-trees, alas.

Comment 9

[Brian Crowder]

Attachment: closer

This is more like the right patch.  Unfortunately, for short scripts, we're way worse-off tracing (because of setup/breakdown) than simply running through the interpreter.  We need to learn what bug 516614 has to say about this in order to develop a heuristic for determining when it is worthwhile to JIT over such code and when it isn't.

Comment 10

[Boris Zbarsky [:bzbarsky]]

So what does the data look like?  What does it look like if we restrict to scripts that call a function?

Comment 11

[Boris Zbarsky [:bzbarsky]]

And in particular, how common is this sort of pattern:

  function foo() {
   // Not very loopy but somewhat slow body
  }
  setInterval(foo, 0);

or equivalent with setTimeout?

Comment 12

[Brian Crowder]

bug 516614 has more details about what sorts of callbacks are getting run when.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Yes, I read that bug.  I can't make sense of the data there, and specifically it doesn't answer my question from comment 11.

Comment 14

[Brian Crowder]

From what I saw, that pattern is very common, but foo() is often a very short function (a handful of spidermonkey opcodes being executed per-run), and in the case of chrome code foo() is actually often an empty function.  The empty-function issue has been patched (I'm not sure how much benefit we saw from that?  Anyone else?), and as I understand it, dvander has some work on his TODO list that will make the timer -> foo() setup/shutdown time less expensive (I don't know what bug that is).  After dvander's work is complete, it should be easy enough to come up with a heuristic for deciding between traced and interpreted code for callbacks like these.

That's my current understanding of our situation, at least.  Hopefully gal or dvander can weigh in.  Also, can you clarify what you aren't discovering from the data in the other bug that you'd like to know?

Thanks

Comment 15

[Boris Zbarsky [:bzbarsky]]

The only data in the other bug is comment 7, right?

That tells me the distribution of opcode lengths, but nothing that tells me what we could to to efficiently detect the large opcode lengths, or to efficiently detect things that correlate well with large opcode lengths.  Or even what opcode lengths ought to be wins when traced (62 opcodes, I'm looking at you).

Comment 16

[Brian Crowder]

Might come back to this later; but in the meantime, back in the pool!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Obsolete with the removal of tracejit.