Add Profiler::DumpStackTrace

Add Profiler::DumpStackTrace

BUG=
R=regis@google.com, srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/7abafbea201c3a8454a7eb2696322b34016c0575

[Cutch, 2016-04-25 19:42:19]

Description was changed from

==========
Add Profiler::DumpStackTrace

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

to

==========
Add Profiler::DumpStackTrace

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

[Cutch, 2016-04-25 19:42:19]

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

[regis, 2016-04-25 20:01:43]

LGTM

[srdjan, 2016-04-25 20:05:55]

Thanks!

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

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:331: static void DumpStackFrame(intptr_t frame, uword pc)
{
maybe s/frame/frame_index/ ... also below

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:368: if (sample_ != NULL) {
I find using positive test (in most cases) to be more readable:
if (sample == NULL) {
  ASSERT(...)
} else {
  ASSERT(...)
  ASSERT(...)
}

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:440: Code& code = Code::Handle();
Can you use zone here (from frame_iterator_ or ProfilerStackWalker)?

[Cutch, 2016-04-25 20:14:54]

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

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:331: static void DumpStackFrame(intptr_t frame, uword pc)
{
On 2016/04/25 20:05:55, srdjan wrote:
> maybe s/frame/frame_index/ ... also below 

Done.

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:368: if (sample_ != NULL) {
On 2016/04/25 20:05:55, srdjan wrote:
> I find using positive test (in most cases) to be more readable:
> if (sample == NULL) {
>   ASSERT(...)
> } else {
>   ASSERT(...)
>   ASSERT(...)
> }

Done.

https://codereview.chromium.org/1919743003/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:440: Code& code = Code::Handle();
On 2016/04/25 20:05:55, srdjan wrote:
> Can you use zone here (from frame_iterator_ or ProfilerStackWalker)?

I've actually re-worked this code here. It was unsafe to allocate a handle in
the common case at all. Now we will only ever use a handle in the
debug-crash-case.

[srdjan, 2016-04-25 20:39:37]

lgtm

[Cutch, 2016-04-25 20:58:19]

Description was changed from

==========
Add Profiler::DumpStackTrace

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

to

==========
Add Profiler::DumpStackTrace

BUG=
R=regis@google.com, srdjan@google.com

Committed:
https://github.com/dart-lang/sdk/commit/7abafbea201c3a8454a7eb2696322b34016c0575
==========

[Cutch, 2016-04-25 20:58:21]

Committed patchset #2 (id:20001) manually as
7abafbea201c3a8454a7eb2696322b34016c0575 (presubmit successful).