Chain samples together to collect long stack traces in profiler

Chain samples together to collect long stack traces in profiler

- Add continuation_index field to profiler sample.
- Add bit to tell if a sample is the head of a chain.
- Add bit to tell if a sample has a chained sample.
- Each sample holds 8 frames.
- Collect up to 8 frames (1 sample) by default.
- Refactor stack walker code to support chaining during stack walking.
- Add unit test collecting 20 stack frames.

R=asiva@google.com

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

[Cutch, 2015-08-01 01:31:54]

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

[siva, 2015-08-04 21:33:10]

lgtm

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

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:269: Sample* next = ReserveSample();
Why not have a private function reserveSampleSlot that return 'cursor' that way
you would avoid the additional IndexOf(next) call below and you could return
At(cursor);

and reserveSample could return At(reserveSampleSlot());

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:279: const uintptr_t samples_addr =
reinterpret_cast<uintptr_t>(samples_);
The name samples_addr makes this code very difficult to read especially stuff
like:

sample_addr - samples_addr

maybe a name like samples_array_addr might make sense?

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:288: #endif
Considering that you have
ASSERT((offset % Sample::instance_size()) == 0);
ASSERT(index >= 0);
ASSERT(index < capacity_);
below, this code within #if defined(DEBUG) ... #endif seems redundant.

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler_tes...
File runtime/vm/profiler_test.cc (right):

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler_tes...
runtime/vm/profiler_test.cc:1133: HANDLESCOPE(isolate);
We are trying to deprecate these APIs that use current isolate so please use the
thread API using Thread::Current()

[Cutch, 2015-08-05 13:49:13]

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

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:269: Sample* next = ReserveSample();
On 2015/08/04 21:33:09, siva wrote:
> Why not have a private function reserveSampleSlot that return 'cursor' that
way
> you would avoid the additional IndexOf(next) call below and you could return
> At(cursor);
> 
> and reserveSample could return At(reserveSampleSlot());

Done.

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:279: const uintptr_t samples_addr =
reinterpret_cast<uintptr_t>(samples_);
On 2015/08/04 21:33:09, siva wrote:
> The name samples_addr makes this code very difficult to read especially stuff
> like:
> 
> sample_addr - samples_addr
> 
> maybe a name like samples_array_addr might make sense?

This is gone now.

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:288: #endif
On 2015/08/04 21:33:09, siva wrote:
> Considering that you have
> ASSERT((offset % Sample::instance_size()) == 0);
> ASSERT(index >= 0);
> ASSERT(index < capacity_);
> below, this code within #if defined(DEBUG) ... #endif seems redundant.

Acknowledged.

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler_tes...
File runtime/vm/profiler_test.cc (right):

https://codereview.chromium.org/1270003002/diff/20001/runtime/vm/profiler_tes...
runtime/vm/profiler_test.cc:1133: HANDLESCOPE(isolate);
On 2015/08/04 21:33:09, siva wrote:
> We are trying to deprecate these APIs that use current isolate so please use
the
> thread API using Thread::Current()

Done here and elsewhere (in this file).

[Cutch, 2015-08-24 18:29:49]

Committed patchset #8 (id:140001) manually as
d61c0ebc04aaff27b1f285316b36dab4c5e98151 (presubmit successful).