[profiler] Log both code and bytecode in heap SFI traversal

[profiler] Log both code and bytecode in heap SFI traversal

The heap traversal for SFI code objects when logging compiled functions
was previously accessing the abstract code of an SFI, logging its
bytecode if it exists or code object otherwise. However, there are some
(rare) cases where an SFI has both bytecode and a non-interpreter code
object -- for example, after baseline tier-up -- in which case we want
to log both, as both could be executing (at different points on the
stack).

BUG=v8:5758
Review-Url: https://codereview.chromium.org/2603333002
Cr-Commit-Position: refs/heads/master@{#42025}
Committed: https://chromium.googlesource.com/v8/v8/+/b8f93dfe9d3b5c7fdb60ad1ab415bfd12f930e16

[Leszek Swirski, 2017-01-03 10:57:20]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-03 10:57:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2017-01-03 11:05:52]

leszeks@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[Leszek Swirski, 2017-01-03 11:05:53]

Hi Ross,

Here's an alternative for https://codereview.chromium.org/2592703002/, LMKWYT.

[commit-bot: I haz the power, 2017-01-03 11:22:58]

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

[commit-bot: I haz the power, 2017-01-03 11:22:59]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-03 12:05:30]

https://codereview.chromium.org/2603333002/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1382
src/log.cc:1382: 
nit - drop the newline

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1394
src/log.cc:1394: if (!sfi->IsInterpreted()) {
This should be HasBaselineCode instead

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1402
src/log.cc:1402: ++compiled_funcs_count;
nit - could you pull the duplicate code out into a helper function which adds
the sfi / abstract code object to sfis / code_objects.

[Leszek Swirski, 2017-01-03 12:22:26]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-03 12:22:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2017-01-03 12:22:46]

https://codereview.chromium.org/2603333002/diff/1/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1382
src/log.cc:1382: 
On 2017/01/03 12:05:30, rmcilroy wrote:
> nit - drop the newline

Done.

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1394
src/log.cc:1394: if (!sfi->IsInterpreted()) {
On 2017/01/03 12:05:30, rmcilroy wrote:
> This should be HasBaselineCode instead

This would change previous behaviour, which also included native functions. I'm
not sure if this would be correct or not though...

https://codereview.chromium.org/2603333002/diff/1/src/log.cc#newcode1402
src/log.cc:1402: ++compiled_funcs_count;
On 2017/01/03 12:05:30, rmcilroy wrote:
> nit - could you pull the duplicate code out into a helper function which adds
> the sfi / abstract code object to sfis / code_objects.

Done.

[rmcilroy, 2017-01-03 12:42:08]

LGTM, thanks.

https://codereview.chromium.org/2603333002/diff/20001/src/log.cc
File src/log.cc (right):

https://codereview.chromium.org/2603333002/diff/20001/src/log.cc#newcode1396
src/log.cc:1396: if (!sfi->IsInterpreted()) {
We might want to replace this with HasBaselineCode() || is_native(), but for now
this works for me.

[Leszek Swirski, 2017-01-03 12:43:34]

The CQ bit was unchecked by leszeks@chromium.org

[Leszek Swirski, 2017-01-03 12:43:35]

The CQ bit was checked by leszeks@chromium.org

[commit-bot: I haz the power, 2017-01-03 12:43:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-03 12:48:06]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483447415086220,
"parent_rev": "0b10c04473587aefc6ef5414473a65aa1707440a", "commit_rev":
"b8f93dfe9d3b5c7fdb60ad1ab415bfd12f930e16"}

[commit-bot: I haz the power, 2017-01-03 12:48:13]

Description was changed from

==========
[profiler] Log both code and bytecode in heap SFI traversal

The heap traversal for SFI code objects when logging compiled functions
was previously accessing the abstract code of an SFI, logging its
bytecode if it exists or code object otherwise. However, there are some
(rare) cases where an SFI has both bytecode and a non-interpreter code
object -- for example, after baseline tier-up -- in which case we want
to log both, as both could be executing (at different points on the
stack).

BUG=v8:5758
==========

to

==========
[profiler] Log both code and bytecode in heap SFI traversal

The heap traversal for SFI code objects when logging compiled functions
was previously accessing the abstract code of an SFI, logging its
bytecode if it exists or code object otherwise. However, there are some
(rare) cases where an SFI has both bytecode and a non-interpreter code
object -- for example, after baseline tier-up -- in which case we want
to log both, as both could be executing (at different points on the
stack).

BUG=v8:5758

Review-Url: https://codereview.chromium.org/2603333002
Cr-Commit-Position: refs/heads/master@{#42025}
Committed:
https://chromium.googlesource.com/v8/v8/+/b8f93dfe9d3b5c7fdb60ad1ab415bfd12f9...
==========

[commit-bot: I haz the power, 2017-01-03 12:48:13]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/b8f93dfe9d3b5c7fdb60ad1ab415bfd12f9...