[kernel] Add TokenPosition to AllocateObject, fix some cc tests

[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject
  meaning that kernel more often has a TokenPosition available.
  This might influence profiling, especially it influences some
  of the vm/cc/Profiler* tests.

- Update profiler_service to also be able to find the current token
  via kernel (as opposed to either returning NULL or crashing).
  This makes use of the source code included in the kernel file.

BUG=
R=kmillikin@google.com, vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/4b9f8eb4601c7214993415c879c61fb965179305

[jensj, 2017-06-16 08:33:05]

jensj@google.com changed reviewers:
+ kmillikin@google.com, vegorov@google.com

[Vyacheslav Egorov (Google), 2017-06-16 11:09:18]

lgtm

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:6242: body_fragment +=
AllocateObject(TokenPosition::kNoSource, klass, 0);
I guess this needs some informative position eventually.

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service.cc
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service...
runtime/vm/profiler_service.cc:2695: if (script.kind() == RawScript::kKernelTag)
{
Please update CL description to include description of this change, because it
does not fit "Add TokenPosition to AllocateObject" description.

[jensj, 2017-06-16 11:36:46]

Description was changed from

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

BUG=
==========

to

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject, meaning that kernel more often has a
TokenPosition available. This might influence profiling, especially it
influences some of the vm/cc/Profiler* tests.
- Update profiler_service to also be able to find the current token via kernel
(as opposed to either returning NULL or crashing). This makes use of the source
code included in the kernel file.

BUG=
==========

[jensj, 2017-06-16 11:38:15]

Description was changed from

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject, meaning that kernel more often has a
TokenPosition available. This might influence profiling, especially it
influences some of the vm/cc/Profiler* tests.
- Update profiler_service to also be able to find the current token via kernel
(as opposed to either returning NULL or crashing). This makes use of the source
code included in the kernel file.

BUG=
==========

to

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject
  meaning that kernel more often has a TokenPosition available.
  This might influence profiling, especially it influences some
  of the vm/cc/Profiler* tests.

- Update profiler_service to also be able to find the current token
  via kernel (as opposed to either returning NULL or crashing).
  This makes use of the source code included in the kernel file.

BUG=
==========

[jensj, 2017-06-16 11:39:07]

Thanks!
I have attempted to update the description.

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:6242: body_fragment +=
AllocateObject(TokenPosition::kNoSource, klass, 0);
On 2017/06/16 11:09:18, Vyacheslav Egorov (Google) wrote:
> I guess this needs some informative position eventually. 

I'll give parser.cc an extra look to see if I can find which position the
source-based pipeline uses.

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service.cc
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service...
runtime/vm/profiler_service.cc:2695: if (script.kind() == RawScript::kKernelTag)
{
On 2017/06/16 11:09:18, Vyacheslav Egorov (Google) wrote:
> Please update CL description to include description of this change, because it
> does not fit "Add TokenPosition to AllocateObject" description. 

"fix some cc tests" isn't descriptive enough?

[jensj, 2017-06-16 11:55:33]

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:6242: body_fragment +=
AllocateObject(TokenPosition::kNoSource, klass, 0);
On 2017/06/16 11:39:07, jensj wrote:
> On 2017/06/16 11:09:18, Vyacheslav Egorov (Google) wrote:
> > I guess this needs some informative position eventually. 
> 
> I'll give parser.cc an extra look to see if I can find which position the
> source-based pipeline uses.

Actually, this gives us the excellent exception
```
'null': Switch case fall-through at line null.
```

So I'll create a new CL for fixing that anyways.

[Kevin Millikin (Google), 2017-06-21 06:50:43]

LGTM.

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service.cc
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/2944433003/diff/1/runtime/vm/profiler_service...
runtime/vm/profiler_service.cc:2695: if (script.kind() == RawScript::kKernelTag)
{
I would probably factor out the common code from the two cases and just put the
different parts in the conditional branches:

String& str = String::Handle(zone);
if (script.kind() == ...) {
  ...
  str = script.GetSnippet(...);
} else {
  const TokenStream& ...
  if (!token_stream.IsNull()) {
    TokenStream::Iterator ...
    str = iterator.CurrentLiteral());
  }
}

return str.IsNull() ? NULL : str.ToCString();

[jensj, 2017-06-21 07:41:21]

Description was changed from

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject
  meaning that kernel more often has a TokenPosition available.
  This might influence profiling, especially it influences some
  of the vm/cc/Profiler* tests.

- Update profiler_service to also be able to find the current token
  via kernel (as opposed to either returning NULL or crashing).
  This makes use of the source code included in the kernel file.

BUG=
==========

to

==========
[kernel] Add TokenPosition to AllocateObject, fix some cc tests

- Add TokenPosition to AllocateObject
  meaning that kernel more often has a TokenPosition available.
  This might influence profiling, especially it influences some
  of the vm/cc/Profiler* tests.

- Update profiler_service to also be able to find the current token
  via kernel (as opposed to either returning NULL or crashing).
  This makes use of the source code included in the kernel file.

BUG=
R=kmillikin@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/4b9f8eb4601c7214993415c879c61fb965179305
==========

[jensj, 2017-06-21 07:41:22]

Committed patchset #1 (id:1) manually as
4b9f8eb4601c7214993415c879c61fb965179305 (presubmit successful).