Support profiler code transition tags

Support profiler code transition tags

- Optionally insert tags when transitioning between unoptimized, optimized, and inlined frames.
- Extend inlined function unit test to verify code transition tags.

R=srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/286c1722f56ac5a9ca655f0cd7a56740afb4c71f

[Cutch, 2015-08-03 17:55:08]

johnmccutchan@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2015-08-04 18:37:16]

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:984: kNumProfileInfoTag,
s/Tag/Kind/

Does the order must be the same as in VMTag (tags.h)? If so you must find a way
to keep those in sync.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1049:
RegisterProfileCodeTag(VMTag::kInlineEndCodeTagId);
Does the order of registration matter?

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1391: const bool inliner = i ==
inlined_functions.length() - 1;
i == (inlined_functions.length() - 1)

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1413: const bool inliner = i ==
inlined_functions.length() - 1;
ditto

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1565: VMTag::kInvalidTagId
If the order must be the same as the other enums, then please find a way to make
sure that is guaranteed.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1642: };
This tag_map initialization is used repeatedly, can you factor it out?

[Cutch, 2015-08-04 21:19:08]

PTAL

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:984: kNumProfileInfoTag,
On 2015/08/04 18:37:15, srdjan wrote:
> s/Tag/Kind/
> 
> Does the order must be the same as in VMTag (tags.h)? If so you must find a
way
> to keep those in sync.

Done. The ordering only matters in the mapping table (I've added asserts).

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1049:
RegisterProfileCodeTag(VMTag::kInlineEndCodeTagId);
On 2015/08/04 18:37:15, srdjan wrote:
> Does the order of registration matter?

No.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1391: const bool inliner = i ==
inlined_functions.length() - 1;
On 2015/08/04 18:37:15, srdjan wrote:
> i == (inlined_functions.length() - 1)

Done.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1413: const bool inliner = i ==
inlined_functions.length() - 1;
On 2015/08/04 18:37:16, srdjan wrote:
> ditto

Done.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1565: VMTag::kInvalidTagId
On 2015/08/04 18:37:16, srdjan wrote:
> If the order must be the same as the other enums, then please find a way to
make
> sure that is guaranteed.

Done.

https://codereview.chromium.org/1268803006/diff/30001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1642: };
On 2015/08/04 18:37:16, srdjan wrote:
> This tag_map initialization is used repeatedly, can you factor it out?

Done.

[srdjan, 2015-08-04 21:24:46]

lgtm

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1558: uword tag_map[] = {
'static uword tag_map', so that it does not get computed/assigned every time?

Is this better than having a switch statement? Switch statement would remove the
need for ASSERT's and may be actually compiled to a table lookup.

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1948: bool TagsEnabled(intptr_t extra_tags_bits)
{
const

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2263: PrintJSONImpl(isolate, stream, tag_order,
0, &filter);
kNone instead of 0?

[Cutch, 2015-08-04 22:00:58]

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1558: uword tag_map[] = {
On 2015/08/04 21:24:46, srdjan wrote:
> 'static uword tag_map', so that it does not get computed/assigned every time?
> 
> Is this better than having a switch statement? Switch statement would remove
the
> need for ASSERT's and may be actually compiled to a table lookup.

Done.

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:1948: bool TagsEnabled(intptr_t extra_tags_bits)
{
On 2015/08/04 21:24:46, srdjan wrote:
> const

Done.

https://codereview.chromium.org/1268803006/diff/50001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2263: PrintJSONImpl(isolate, stream, tag_order,
0, &filter);
On 2015/08/04 21:24:46, srdjan wrote:
> kNone instead of 0?

Done.

[Cutch, 2015-08-04 22:02:55]

Committed patchset #5 (id:70001) manually as
286c1722f56ac5a9ca655f0cd7a56740afb4c71f (presubmit successful).