StackFrameIterator looks up code object earlier to decide frame type.

Refactor the StackFrameIterator::ComputeType() method to look up the code object (if any) before looking at the magic markers.

This will allow per-code-kind logic more easily in the future (e.g. for WASM).

BUG=

Committed: https://crrev.com/5c35f5e302994e5dde6a76bcb5823a1cd5a42ed2
Cr-Commit-Position: refs/heads/master@{#30912}

[titzer, 2015-09-22 15:58:49]

titzer@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[titzer, 2015-09-22 15:58:49]

PTAL

[Hannes Payer (out of office), 2015-09-22 16:31:23]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2015-09-22 16:31:24]

https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:3036: bool LargeObjectSpace::Contains(Address address) {
We have FindPage or FindObject (depending on what you want) which tells you if
an address is in LO space.

[titzer, 2015-09-23 09:33:57]

https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:3036: bool LargeObjectSpace::Contains(Address address) {
On 2015/09/22 16:31:23, Hannes Payer wrote:
> We have FindPage or FindObject (depending on what you want) which tells you if
> an address is in LO space.

Ok, I can implement this method in terms of FindPage() which uses a hashtable
inside instead of the linear search. How many pages do we typically have in the
large object space? BTW do we really need a hashtable for that or could a binary
search be fine?

[titzer, 2015-09-23 10:53:44]

On 2015/09/23 09:33:57, titzer wrote:
> https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc
> File src/heap/spaces.cc (right):
> 
>
https://codereview.chromium.org/1350763004/diff/60001/src/heap/spaces.cc#newc...
> src/heap/spaces.cc:3036: bool LargeObjectSpace::Contains(Address address) {
> On 2015/09/22 16:31:23, Hannes Payer wrote:
> > We have FindPage or FindObject (depending on what you want) which tells you
if
> > an address is in LO space.
> 
> Ok, I can implement this method in terms of FindPage() which uses a hashtable
> inside instead of the linear search. How many pages do we typically have in
the
> large object space? BTW do we really need a hashtable for that or could a
binary
> search be fine?

PTAL

[Hannes Payer (out of office), 2015-09-23 11:53:05]

LGTM for heap/*

[Michael Starzinger, 2015-09-24 08:30:12]

LGTM if comments are addressed.

https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode435
src/frames.cc:435: if (code_obj) {
nit: "code_obj != nullptr"

https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode461
src/frames.cc:461: if (Smi::cast(marker)->value() > 100) {
nit: Magic 100 is being magic, can we use "Code::NUMBER_OF_KINDS" instead?

https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode462
src/frames.cc:462: base::OS::DebugBreak();
nit: Can we use FATAL("some descriptive message") here instead?

[titzer, 2015-09-24 12:12:53]

On 2015/09/24 08:30:12, Michael Starzinger wrote:
> LGTM if comments are addressed.
> 
> https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc
> File src/frames.cc (right):
> 
> https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode435
> src/frames.cc:435: if (code_obj) {
> nit: "code_obj != nullptr"
> 

Done.

> https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode461
> src/frames.cc:461: if (Smi::cast(marker)->value() > 100) {
> nit: Magic 100 is being magic, can we use "Code::NUMBER_OF_KINDS" instead?
> 
> https://codereview.chromium.org/1350763004/diff/80001/src/frames.cc#newcode462
> src/frames.cc:462: base::OS::DebugBreak();
> nit: Can we use FATAL("some descriptive message") here instead?

I just removed it; it was leftover debugging code. Again, I'm not entirely sure
that can't happen because of the stupid profiler.

[titzer, 2015-09-24 12:49:53]

The CQ bit was checked by titzer@chromium.org

[titzer, 2015-09-24 12:49:53]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org,
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1350763004/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-09-24 12:50:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1350763004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1350763004/100001

[commit-bot: I haz the power, 2015-09-24 12:51:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-24 12:52:08]

Patchset 6 (id:??) landed as
https://crrev.com/5c35f5e302994e5dde6a76bcb5823a1cd5a42ed2
Cr-Commit-Position: refs/heads/master@{#30912}