Adds support for collecting statistics about code and its metadata.

Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N

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

[mythria, 2016-05-23 08:59:53]

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

[commit-bot: I haz the power, 2016-05-23 08:59:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997363002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997363002/20001

[mythria, 2016-05-23 09:01:22]

mythria@chromium.org changed reviewers:
+ hpayer@chromium.org, rmcilroy@chromium.org

[mythria, 2016-05-23 09:01:23]

PTAL.

Thanks,
Mythri

[commit-bot: I haz the power, 2016-05-23 09:05:35]

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

[commit-bot: I haz the power, 2016-05-23 09:05:36]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/7685)

[mythria, 2016-05-23 09:50:42]

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

[commit-bot: I haz the power, 2016-05-23 09:50:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997363002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997363002/40001

[rmcilroy, 2016-05-23 10:16:30]

Looks reasonable. A couple of comments and I'll leave Hannes to approve.

https://codereview.chromium.org/1997363002/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1997363002/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:741: DEFINE_BOOL(code_and_metadata_stats, false,
Do we need this flag? If this is only collected when you request it is there any
reason to disable it - it shouldn't affect performance unless you call the API
should it?

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2848: // RelocInformation.
Bytecode arrays will never have RelocInfo, so this comment is incorrect.

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2889: static void RecordMetadataStatistics(AbstractCode*
abstract_code,
This is code and metadata sizes isn't it? If so, update the function name.

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2891: int size = abstract_code->InstructionAndMetadataSize();
I don't like the name InstructionAndMetadataSize(). How about
SizeIncludingMetadata (also for both of the subclasses)?

[commit-bot: I haz the power, 2016-05-23 10:22:57]

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

[commit-bot: I haz the power, 2016-05-23 10:22:58]

Dry run: This issue passed the CQ dry run.

[mythria, 2016-05-23 11:21:03]

Description was changed from

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the entire heap is scanned to find any
code/bytecode array objects. This could be slow and hence it should be used
with caution. The code statistics are only collected when
FLAG_code_and_metadata_stats is enabled.


BUG=v8:5019
LOG=N
==========

to

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N
==========

[mythria, 2016-05-23 11:27:51]

Thanks Ross, I addressed your comments. 

Thanks,
Mythri

https://codereview.chromium.org/1997363002/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/1997363002/diff/40001/src/flag-definitions.h#...
src/flag-definitions.h:741: DEFINE_BOOL(code_and_metadata_stats, false,
On 2016/05/23 10:16:30, rmcilroy wrote:
> Do we need this flag? If this is only collected when you request it is there
any
> reason to disable it - it shouldn't affect performance unless you call the API
> should it?

Yes, it collects code statistics only when requested, so does not affect
performance. I am removing this flag. Done.

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2848: // RelocInformation.
On 2016/05/23 10:16:30, rmcilroy wrote:
> Bytecode arrays will never have RelocInfo, so this comment is incorrect.

Thanks. I removed the comment. Done.

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2889: static void RecordMetadataStatistics(AbstractCode*
abstract_code,
On 2016/05/23 10:16:30, rmcilroy wrote:
> This is code and metadata sizes isn't it? If so, update the function name.

Done.

https://codereview.chromium.org/1997363002/diff/40001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2891: int size = abstract_code->InstructionAndMetadataSize();
On 2016/05/23 10:16:30, rmcilroy wrote:
> I don't like the name InstructionAndMetadataSize(). How about
> SizeIncludingMetadata (also for both of the subclasses)?

 Done.

[Hannes Payer (out of office), 2016-05-25 19:16:20]

LGTM

https://codereview.chromium.org/1997363002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/1997363002/diff/60001/src/compiler.cc#newcode451
src/compiler.cc:451: return info->code()->SizeIncludingMetadata();
Drop the else statement.

https://codereview.chromium.org/1997363002/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1997363002/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2846: static void CollectCodeCommentStatistics(HeapObject*
obj, Isolate* isolate) {
It is a bit strange that the statistics functions are part of the space. We
could factor that out into a different statistics class. Let's do that in a
follow-up CL. WDYT?

[mythria, 2016-05-26 08:51:06]

Thanks Hannes. I addressed the nits and I agree that its better to have a
different class for collecting statistics. I will do that in a follow up cl.

Regards,
Mythri

https://codereview.chromium.org/1997363002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/1997363002/diff/60001/src/compiler.cc#newcode451
src/compiler.cc:451: return info->code()->SizeIncludingMetadata();
On 2016/05/25 19:16:20, Hannes Payer wrote:
> Drop the else statement.

Done.

https://codereview.chromium.org/1997363002/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1997363002/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2846: static void CollectCodeCommentStatistics(HeapObject*
obj, Isolate* isolate) {
On 2016/05/25 19:16:20, Hannes Payer wrote:
> It is a bit strange that the statistics functions are part of the space. We
> could factor that out into a different statistics class. Let's do that in a
> follow-up CL. WDYT?

Yes, I think it is better to have a different class to collect statistics. I
will address this in a different cl, so it won't block adding benchmarks.

[mythria, 2016-05-26 08:51:18]

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

[commit-bot: I haz the power, 2016-05-26 08:51:23]

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

[commit-bot: I haz the power, 2016-05-26 09:25:14]

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

[commit-bot: I haz the power, 2016-05-26 09:25:15]

Dry run: This issue passed the CQ dry run.

[commit-bot: I haz the power, 2016-05-26 09:25:23]

Dry run: None

[mythria, 2016-05-26 09:44:25]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-05-26 09:44:26]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1997363002/#ps80001
(title: "Addressed comments from Hannes.")

[commit-bot: I haz the power, 2016-05-26 09:44:34]

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

[commit-bot: I haz the power, 2016-05-26 09:46:43]

Description was changed from

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N
==========

to

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N
==========

[commit-bot: I haz the power, 2016-05-26 09:46:45]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-26 09:47:36]

Description was changed from

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N
==========

to

==========
Adds support for collecting statistics about code and its metadata.

Adds an API to request the total size of code/bytecode and their associated
metadata in the heap. When requested, the code_space, old_space and
large_object_space  is scanned to find any code/bytecode array objects.
This could be slow and hence it should be used with caution.

BUG=v8:5019
LOG=N

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

[commit-bot: I haz the power, 2016-05-26 09:47:37]

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