[Interpreter] Enable runtime profiler support for Ignition.

[Interpreter] Enable runtime profiler support for Ignition.

Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.

Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N

Committed: https://crrev.com/b62bf1e6fb7061f89b9ba0a06493109042b86978
Cr-Commit-Position: refs/heads/master@{#34166}

[rmcilroy, 2016-02-17 17:12:44]

Description was changed from

==========
[Interpreter] Enable runtime profiler support for Ignition.

Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to decrement this
counter on returns or backward jumps. Modifies RuntimeProfiler
to track ticks for interpreted frames. Currently we use the
SharedFunctionInfo::profiler_ticks() instead of adding another
to tick field to avoid adding another field to BytecodeArray
since SharedFunctionInfo::profiler_ticks() is only used by
Crankshaft otherwise so we shouldn't need both for Ignition+TF.

BUG=v8:4689
LOG=N
==========

to

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for
==========

[rmcilroy, 2016-02-17 17:16:25]

rmcilroy@chromium.org changed reviewers:
+ jkummerow@chromium.org, mstarzinger@chromium.org

[rmcilroy, 2016-02-17 17:16:25]

Jakob / Michi, PTAL. Hopefully we are on the same page and this is what you had
in mind when you were discussing things over dinner.

Enabling this flushes out an issue in deopting (deopting a function where the
accumulator value needs to be materialized). I'll work on that and land the fix
for that before this, but thought I would through this out there for you to have
a look at.

[rmcilroy, 2016-02-17 17:17:27]

Description was changed from

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for
==========

to

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N
==========

[Jakob Kummerow, 2016-02-17 17:45:36]

LGTM with a few nits.

Higher-level nit: I tend to prefer the term "profiling budget" (or "profiler
budget") over "profiling counter" for better distinction from "tick counter". I
don't feel strongly about it though.

https://codereview.chromium.org/1707693003/diff/20001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:68: V(Int32GreaterThan)                      
  \
nit: put this to the other Int32* please (so at least the prefixes are sorted
alphabetically; I'm fine with the suffixes being sorted by belonging-together)

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.h:158: // Updates the profiling counter by
|weight| and calls InteruptCheck if
nit: s/Interupt/Interrupt/

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.h:36: // Returns the interrupt budget which shoud be
used for the profiler counter.
nit: s/shoud/should/

https://codereview.chromium.org/1707693003/diff/20001/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):

https://codereview.chromium.org/1707693003/diff/20001/src/runtime-profiler.cc...
src/runtime-profiler.cc:246: // TODO(rmcilroy): Consider whether we should
optimize small functions when
Try to avoid that; it's a SunSpider hack.

[rmcilroy, 2016-02-18 16:05:57]

Opps, forgot to hit publish on this...

Michi, would are you wanting to take a look before I commit?


> Higher-level nit: I tend to prefer the term "profiling budget" (or "profiler
> budget") over "profiling counter" for better distinction from "tick counter".
I
> don't feel strongly about it though.

Changed to interrupt_budget as discussed offline.

https://codereview.chromium.org/1707693003/diff/20001/src/compiler/code-stub-...
File src/compiler/code-stub-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/compiler/code-stub-...
src/compiler/code-stub-assembler.h:68: V(Int32GreaterThan)                      
  \
On 2016/02/17 17:45:36, Jakob wrote:
> nit: put this to the other Int32* please (so at least the prefixes are sorted
> alphabetically; I'm fine with the suffixes being sorted by belonging-together)

Done.

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.h:158: // Updates the profiling counter by
|weight| and calls InteruptCheck if
On 2016/02/17 17:45:36, Jakob wrote:
> nit: s/Interupt/Interrupt/

Done.

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
File src/interpreter/interpreter.h (right):

https://codereview.chromium.org/1707693003/diff/20001/src/interpreter/interpr...
src/interpreter/interpreter.h:36: // Returns the interrupt budget which shoud be
used for the profiler counter.
On 2016/02/17 17:45:36, Jakob wrote:
> nit: s/shoud/should/

Done.

https://codereview.chromium.org/1707693003/diff/20001/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):

https://codereview.chromium.org/1707693003/diff/20001/src/runtime-profiler.cc...
src/runtime-profiler.cc:246: // TODO(rmcilroy): Consider whether we should
optimize small functions when
On 2016/02/17 17:45:36, Jakob wrote:
> Try to avoid that; it's a SunSpider hack.

Acknowledged.

[Michael Starzinger, 2016-02-18 17:55:26]

LGTM on the approach itself and on the plumbing around it, didn't look at
runtime-profiler.cc at all, Jakob is the expert on that one.

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.cc (right):

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.cc:467: // TODO(rmcilroy): Deal with self
optimization of primitive functions.
Just for posterity: Unless we have data supporting the need for the different
weighing when CompilationInfo::ShouldSelfOptimize is set, I would vote for not
porting that over to Ignition. I tried to remove it from full-codegen a while
ago and only caused tankage on a very small set of benchmarks.

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.h:158: // Updates the profiling counter by
|weight| and calls InteruptCheck if
nit: s/InteruptCheck/Runtime_Interrupt/ ... or Runtime::kInterrupt

[rmcilroy, 2016-02-19 12:32:32]

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.cc (right):

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.cc:467: // TODO(rmcilroy): Deal with self
optimization of primitive functions.
On 2016/02/18 17:55:26, Michael Starzinger wrote:
> Just for posterity: Unless we have data supporting the need for the different
> weighing when CompilationInfo::ShouldSelfOptimize is set, I would vote for not
> porting that over to Ignition. I tried to remove it from full-codegen a while
> ago and only caused tankage on a very small set of benchmarks.

Ack. Updated comment.

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/1707693003/diff/60001/src/interpreter/interpr...
src/interpreter/interpreter-assembler.h:158: // Updates the profiling counter by
|weight| and calls InteruptCheck if
On 2016/02/18 17:55:26, Michael Starzinger wrote:
> nit: s/InteruptCheck/Runtime_Interrupt/ ... or Runtime::kInterrupt

Done.

[rmcilroy, 2016-02-19 12:32:37]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-19 12:32:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1707693003/#ps80001
(title: "Address comments")

[commit-bot: I haz the power, 2016-02-19 12:32:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1707693003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1707693003/80001

[commit-bot: I haz the power, 2016-02-19 12:38:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-19 12:38:02]

Try jobs failed on following builders:
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[rmcilroy, 2016-02-19 14:40:06]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-19 14:40:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1707693003/#ps100001
(title: "Fix InterpreterAssembler unittests.")

[commit-bot: I haz the power, 2016-02-19 14:40:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1707693003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1707693003/100001

[commit-bot: I haz the power, 2016-02-19 14:47:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-19 14:47:26]

Try jobs failed on following builders:
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[rmcilroy, 2016-02-19 15:08:38]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-19 15:08:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1707693003/#ps120001
(title: "Reorder BytecodeArray fields to avoid unaligned access on MIPS.")

[commit-bot: I haz the power, 2016-02-19 15:08:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1707693003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1707693003/120001

[commit-bot: I haz the power, 2016-02-19 15:49:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-19 15:50:00]

Try jobs failed on following builders:
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/1654)
  v8_linux_dbg_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[rmcilroy, 2016-02-19 16:19:29]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-19 16:19:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1707693003/#ps140001
(title: "Fix for --debug-code case")

[commit-bot: I haz the power, 2016-02-19 16:19:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1707693003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1707693003/140001

[commit-bot: I haz the power, 2016-02-19 16:52:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-19 16:52:43]

Try jobs failed on following builders:
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/1659)
  v8_linux_dbg_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)

[rmcilroy, 2016-02-19 17:25:19]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-02-19 17:25:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1707693003/#ps160001
(title: "Really fix --debug-code")

[commit-bot: I haz the power, 2016-02-19 17:25:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1707693003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1707693003/160001

[commit-bot: I haz the power, 2016-02-19 18:47:00]

Description was changed from

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N
==========

to

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N
==========

[commit-bot: I haz the power, 2016-02-19 18:47:01]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-19 18:47:23]

Description was changed from

==========
[Interpreter] Enable runtime profiler support for Ignition.
 
Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.
    
Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N
==========

to

==========
[Interpreter] Enable runtime profiler support for Ignition.

Adds a profiling counter to each BytecodeArray object, and adds
code to Jump and Return bytecode handlers to update this
counter by the size of the jump or the distance from the return
to the start of the function. This is more accurate than fullcodegen's
approach since it takes forward jumps into account as well as back-edges.

Modifies RuntimeProfiler to track ticks for interpreted frames.
Currently we use the SharedFunctionInfo::profiler_ticks() instead
of adding another to tick field to avoid adding another field to
BytecodeArray since SharedFunctionInfo::profiler_ticks() is only
used by Crankshaft otherwise so we shouldn't need both for

BUG=v8:4689
LOG=N

Committed: https://crrev.com/b62bf1e6fb7061f89b9ba0a06493109042b86978
Cr-Commit-Position: refs/heads/master@{#34166}
==========

[commit-bot: I haz the power, 2016-02-19 18:47:24]

Patchset 9 (id:??) landed as
https://crrev.com/b62bf1e6fb7061f89b9ba0a06493109042b86978
Cr-Commit-Position: refs/heads/master@{#34166}