Iterate over PcDescriptors only via iterators, not via an index. (preparation for more compression …

Iterate over PcDescriptors only via iterators, not via an index. (preparation for more compression of PcDescriptors).

R=asiva@google.com

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

[srdjan, 2014-06-27 17:07:41]

On 2014/06/27 16:59:08, srdjan wrote:

FYI, planned work includes:

- NextPc(uword pc) => iterates until the pc is found, returns record containing
the pc or null (similar NextKind, etc).
- kind field becomes a bitmap, so that we can merge several records that differ
only in kind.

[siva, 2014-07-01 18:37:11]

LGTM with one comment about the need to move the kind description from Object to
RawObject

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/code_...
File runtime/vm/code_descriptors_test.cc (right):

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/code_...
runtime/vm/code_descriptors_test.cc:260: PcDescriptors::Iterator
iter(descriptors);
Not for this CL but it might make things easier to have a kind
in the iterators.

e.g:
PcDescriptors::Iterator iter(descriptors,
                             PcDescriptors::kUnoptStaticCall);
const RawPcDescriptors::PcDescriptorRec* rec = iter.Next();
while (rec != NULL) {
   stackmap_table_builder->AddEntry(rec->pc - code.EntryPoint(),
                                    stack_bitmap,
                                    0);
   ++call_count;
}

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/object.h
File runtime/vm/object.h (left):

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/objec...
runtime/vm/object.h:3015: const char* KindAsStr(intptr_t index) const;
Why was it necessary to move the description of kind and these accessors away
from here into RawPcDescriptors ?

[srdjan, 2014-07-07 18:01:26]

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/code_...
File runtime/vm/code_descriptors_test.cc (right):

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/code_...
runtime/vm/code_descriptors_test.cc:260: PcDescriptors::Iterator
iter(descriptors);
On 2014/07/01 18:37:11, siva wrote:
> Not for this CL but it might make things easier to have a kind
> in the iterators.
> 
> e.g:
> PcDescriptors::Iterator iter(descriptors,
>                              PcDescriptors::kUnoptStaticCall);
> const RawPcDescriptors::PcDescriptorRec* rec = iter.Next();
> while (rec != NULL) {
>    stackmap_table_builder->AddEntry(rec->pc - code.EntryPoint(),
>                                     stack_bitmap,
>                                     0);
>    ++call_count;
> }  

I am planning either that, or specifying the kind in the call to Next.

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/object.h
File runtime/vm/object.h (left):

https://chromiumcodereview.appspot.com/356923006/diff/220001/runtime/vm/objec...
runtime/vm/object.h:3015: const char* KindAsStr(intptr_t index) const;
On 2014/07/01 18:37:11, siva wrote:
> Why was it necessary to move the description of kind and these accessors away
> from here into RawPcDescriptors ?

Because PcDescriptorRec is defined in RawPcDescriptors and is used to
store/access the data. In order to map an int8_t to Kind, the Kind enum must be
known in raw_object.h.

[srdjan, 2014-07-07 18:26:01]

Committed patchset #14 manually as r38032 (presubmit successful).

[Ivan Posva, 2014-07-16 08:08:49]

DBC -ip

https://codereview.chromium.org/356923006/diff/230001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/356923006/diff/230001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:366: desc_rec_ = &rec;
What is the lifetime of this PcDescriptorRec? Or asked differently: When is the
desc_rec_ pointer within the ActivationFrame accessed and how to we ensure that
the iterator has not been deleted or has not moved forward since we captured the
value here?

[hausner, 2014-07-16 16:14:08]

https://codereview.chromium.org/356923006/diff/230001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/356923006/diff/230001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:366: desc_rec_ = &rec;
On 2014/07/16 08:08:49, Ivan Posva wrote:
> What is the lifetime of this PcDescriptorRec? Or asked differently: When is
the
> desc_rec_ pointer within the ActivationFrame accessed and how to we ensure
that
> the iterator has not been deleted or has not moved forward since we captured
the
> value here?

Good call. It seems that desc_rec_ is pointing into a RawPcDescriptor object.
Srdjan just confirmed that this is a bug. He'll fix it.