Add runtime and native entry tags

Add runtime and native entry tags

- Split runtime and native entry tags.
- Set runtime / native entry address as vm tag in stubs.
- Add reverse native entry resolver (pc to name) to public API (breaking change).
- Hookup reverse resolver for standalone VM (io + bootstrap + builtin natives).
- Runtime entries are registered with tags.
- Profiler queries for tag names via the reverse native entry resolver.
- Profiler outputs named entry tags as children under parent tag (runtime or native).

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34941

[siva, 2014-04-08 16:27:54]

https://codereview.chromium.org/227433004/diff/10001/runtime/bin/builtin.h
File runtime/bin/builtin.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/bin/builtin.h#ne...
runtime/bin/builtin.h:60: static const char*
NativeReverseLookup(Dart_NativeFunction nf);
Instead of NativeReverseLookup I would call it NativeSymbol

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2216: * Native entry reverse resolution callback.
Native entry symbol lookup callback.

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2219: * can provide a native entry reverse resolver.
This callback is used to map
can provide a callback for mapping a native entry to a symbol. This callback
maps

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2230: typedef const char*
(*Dart_NativeEntryReverseResolver)(Dart_NativeFunction nf);
Should the return value be a const char* or a uint8_t* where the symbol is
returned in UTF8 format?

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2270: Dart_NativeEntryReverseResolver
reverse_resolver);
Dart_NativeEntrySymbol

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1600: }
Does it make sense to start the profiler before the VM isolate is ready as we
are not recording any VM isolate samples.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1604: }
Instead of checking like this wouldn't it be more safe to start the profiler
only after all the initialization is done if FLAG_profile_native_stack is false?

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1637: // frames.
what does this TODO mean (for pure Dart frame walking we must have the last exit
frame info and native frames are not traversed at all).

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h#newcode47
runtime/vm/tags.h:47: V(CompileFunction)                                        
                  \
Can this be made part of DEFINE_RUNTIME_ENTRY i.e the DEFINE_RUNTIME_ENTRY fills
up the table that you want for looking up runtime entries.

[Cutch, 2014-04-09 17:40:59]

https://codereview.chromium.org/227433004/diff/10001/runtime/bin/builtin.h
File runtime/bin/builtin.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/bin/builtin.h#ne...
runtime/bin/builtin.h:60: static const char*
NativeReverseLookup(Dart_NativeFunction nf);
On 2014/04/08 16:27:54, siva wrote:
> Instead of NativeReverseLookup I would call it NativeSymbol

Done.

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2216: * Native entry reverse resolution callback.
On 2014/04/08 16:27:54, siva wrote:
> Native entry symbol lookup callback.

Done.

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2219: * can provide a native entry reverse resolver.
This callback is used to map
On 2014/04/08 16:27:54, siva wrote:
> can provide a callback for mapping a native entry to a symbol. This callback
> maps

Done.

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2230: typedef const char*
(*Dart_NativeEntryReverseResolver)(Dart_NativeFunction nf);
On 2014/04/08 16:27:54, siva wrote:
> Should the return value be a const char* or a uint8_t* where the symbol is
> returned in UTF8 format?

UTF8.

https://codereview.chromium.org/227433004/diff/10001/runtime/include/dart_api...
runtime/include/dart_api.h:2270: Dart_NativeEntryReverseResolver
reverse_resolver);
On 2014/04/08 16:27:54, siva wrote:
> Dart_NativeEntrySymbol

Done.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1600: }
On 2014/04/08 16:27:54, siva wrote:
> Does it make sense to start the profiler before the VM isolate is ready as we
> are not recording any VM isolate samples.

Isolate::SetCurrent expects the profiler to have been initialized. I'd rather
not make SetCurrent check if the profiler is initialized.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1604: }
On 2014/04/08 16:27:54, siva wrote:
> Instead of checking like this wouldn't it be more safe to start the profiler
> only after all the initialization is done if FLAG_profile_native_stack is
false?

See above.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1637: // frames.
On 2014/04/08 16:27:54, siva wrote:
> what does this TODO mean (for pure Dart frame walking we must have the last
exit
> frame info and native frames are not traversed at all).

Slightly misworded but I want to support only collecting Dart frames even
without top_exit_frame_info.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h#newcode47
runtime/vm/tags.h:47: V(CompileFunction)                                        
                  \
On 2014/04/08 16:27:54, siva wrote:
> Can this be made part of DEFINE_RUNTIME_ENTRY i.e the DEFINE_RUNTIME_ENTRY
fills
> up the table that you want for looking up runtime entries.

DEFINE_RUNTIME_ENTRY cannot execute code so I'm not sure how I could dynamically
register runtime entries in the macro.

[siva, 2014-04-09 23:28:11]

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1600: }
I agree we don't want to make Isolate::SetCurrent have to check if the profiler
is initialized.

Maybe we should have a private 'BaseSetCurrent' method (the name could be
something more meaningful), this would have just the ThreadLocal setting
functionality. We call this in Isolate::Init() and then start the profiler once
the isolate is setup. 

On 2014/04/09 17:40:59, Cutch wrote:
> On 2014/04/08 16:27:54, siva wrote:
> > Does it make sense to start the profiler before the VM isolate is ready as
we
> > are not recording any VM isolate samples.
> 
> Isolate::SetCurrent expects the profiler to have been initialized. I'd rather
> not make SetCurrent check if the profiler is initialized.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h#newcode47
runtime/vm/tags.h:47: V(CompileFunction)                                        
                  \
Look at UNIT_TEST_CASE in unit_test.h it creates a static const object and the
constructor of that object runs linking each test case into a list. This list is
then traversed later, we could do something similar to set up a list of runtime
entries which you can then traverse.


On 2014/04/09 17:40:59, Cutch wrote:
> On 2014/04/08 16:27:54, siva wrote:
> > Can this be made part of DEFINE_RUNTIME_ENTRY i.e the DEFINE_RUNTIME_ENTRY
> fills
> > up the table that you want for looking up runtime entries.
> 
> DEFINE_RUNTIME_ENTRY cannot execute code so I'm not sure how I could
dynamically
> register runtime entries in the macro.

[Cutch, 2014-04-10 15:30:27]

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/profiler.cc#n...
runtime/vm/profiler.cc:1600: }
On 2014/04/09 23:28:11, siva wrote:
> I agree we don't want to make Isolate::SetCurrent have to check if the
profiler
> is initialized.
> 
> Maybe we should have a private 'BaseSetCurrent' method (the name could be
> something more meaningful), this would have just the ThreadLocal setting
> functionality. We call this in Isolate::Init() and then start the profiler
once
> the isolate is setup. 
> 
> On 2014/04/09 17:40:59, Cutch wrote:
> > On 2014/04/08 16:27:54, siva wrote:
> > > Does it make sense to start the profiler before the VM isolate is ready as
> we
> > > are not recording any VM isolate samples.
> > 
> > Isolate::SetCurrent expects the profiler to have been initialized. I'd
rather
> > not make SetCurrent check if the profiler is initialized.
> 

Done.

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/10001/runtime/vm/tags.h#newcode47
runtime/vm/tags.h:47: V(CompileFunction)                                        
                  \
On 2014/04/09 23:28:11, siva wrote:
> Look at UNIT_TEST_CASE in unit_test.h it creates a static const object and the
> constructor of that object runs linking each test case into a list. This list
is
> then traversed later, we could do something similar to set up a list of
runtime
> entries which you can then traverse.
> 
> 
> On 2014/04/09 17:40:59, Cutch wrote:
> > On 2014/04/08 16:27:54, siva wrote:
> > > Can this be made part of DEFINE_RUNTIME_ENTRY i.e the DEFINE_RUNTIME_ENTRY
> > fills
> > > up the table that you want for looking up runtime entries.
> > 
> > DEFINE_RUNTIME_ENTRY cannot execute code so I'm not sure how I could
> dynamically
> > register runtime entries in the macro.
> 

Done.

[siva, 2014-04-10 17:58:22]

lgtm

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/stack_frame.h
File runtime/vm/stack_frame.h (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/stack_frame.h...
runtime/vm/stack_frame.h:245: explicit DartFrameIterator(uword fp, uword sp,
uword pc)
explicit not needed as it has more than one parameter.

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.cc
File runtime/vm/tags.cc (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.cc#newco...
runtime/vm/tags.cc:75:
reinterpret_cast<RegisteredRuntimeEntry*>(malloc(sizeof(*entry)));
Do we need a new type RegisteredRuntimeEntry here which needs to be malloced, I
was wondering if we could add a 'next' field to RuntimeEntry itself and hook it
up here.

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.h#newcode9
runtime/vm/tags.h:9: class RuntimeEntry;
Repeated again below, I guess it is not needed here.

[Cutch, 2014-04-10 20:17:31]

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/stack_frame.h
File runtime/vm/stack_frame.h (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/stack_frame.h...
runtime/vm/stack_frame.h:245: explicit DartFrameIterator(uword fp, uword sp,
uword pc)
On 2014/04/10 17:58:22, siva wrote:
> explicit not needed as it has more than one parameter.

Done.

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.cc
File runtime/vm/tags.cc (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.cc#newco...
runtime/vm/tags.cc:75:
reinterpret_cast<RegisteredRuntimeEntry*>(malloc(sizeof(*entry)));
On 2014/04/10 17:58:22, siva wrote:
> Do we need a new type RegisteredRuntimeEntry here which needs to be malloced,
I
> was wondering if we could add a 'next' field to RuntimeEntry itself and hook
it
> up here.

Done.

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.h
File runtime/vm/tags.h (right):

https://codereview.chromium.org/227433004/diff/70001/runtime/vm/tags.h#newcode9
runtime/vm/tags.h:9: class RuntimeEntry;
On 2014/04/10 17:58:22, siva wrote:
> Repeated again below, I guess it is not needed here.

Done.

[Cutch, 2014-04-10 20:27:00]

Committed patchset #7 manually as r34941 (presubmit successful).