Add dead CodeRegionTable for tracking overwritten Dart code

Create a shadow table to track CodeRegions of reused code address space.

R=asiva@google.com, turnidge@google.com

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

[siva, 2014-03-13 17:12:38]

lgtm

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

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:168: start_ = OS::GetCurrentTimeMillis();
This also should be done conditionally under the flag
if (FLAG_trace_profiled_isolates)..

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:220: kOverwrittenCode,  // Dead Dart code that has been
overwritten by kDartCode.
I would call this kReusedCode as it is a region that has been deleted and
reused, overwritten sounds like a bug.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:780: heap_(isolate->heap()) {
Why not store the isolate as a field in this class instead of heap_ that way you
would not have to do Isolate::Current() in some of the code below (e.g:
LookupCode, Code::Handle ..) and heap would just be isolate->heap()

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:800: */
Why do you want to keep this commented out code around?

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:894: if (heap_->CodeContains(pc) ||
vm_isolate_heap_->CodeContains(pc)) {
Wouldn't it be more efficient to do this check first and then if that is true do
a Code::LookupCode(pc) followed by LookupCodeInVmIsolate(pc)...

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:967: intptr_t FindFinalIndex(uword pc, int64_t timestamp)
{
) const {

[Cutch, 2014-03-13 17:33:57]

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

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:168: start_ = OS::GetCurrentTimeMillis();
On 2014/03/13 17:12:38, siva wrote:
> This also should be done conditionally under the flag
> if (FLAG_trace_profiled_isolates)..

Done.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:220: kOverwrittenCode,  // Dead Dart code that has been
overwritten by kDartCode.
On 2014/03/13 17:12:38, siva wrote:
> I would call this kReusedCode as it is a region that has been deleted and
> reused, overwritten sounds like a bug.

Done.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:780: heap_(isolate->heap()) {
On 2014/03/13 17:12:38, siva wrote:
> Why not store the isolate as a field in this class instead of heap_ that way
you
> would not have to do Isolate::Current() in some of the code below (e.g:
> LookupCode, Code::Handle ..) and heap would just be isolate->heap()

Done.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:800: */
On 2014/03/13 17:12:38, siva wrote:
> Why do you want to keep this commented out code around?

Removed.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:894: if (heap_->CodeContains(pc) ||
vm_isolate_heap_->CodeContains(pc)) {
On 2014/03/13 17:12:38, siva wrote:
> Wouldn't it be more efficient to do this check first and then if that is true
do
> a Code::LookupCode(pc) followed by LookupCodeInVmIsolate(pc)...

Done.

https://codereview.chromium.org/197803004/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:967: intptr_t FindFinalIndex(uword pc, int64_t timestamp)
{
On 2014/03/13 17:12:38, siva wrote:
> ) const {

Done.

[Cutch, 2014-03-13 17:35:01]

Todd,

Can you please review the service and Observatory changes?

Thanks,
John

[turnidge, 2014-03-13 18:09:09]

lgtm w/ comments.

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/code_view.html (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/code_view.html:29: }
To keep in sync with my changes I'm making now...

Rename .memberBold -> .memberHeader

Combine rules for .member and .memberHeader since they are currently the same:

.member, .memberHeader { ...

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/cache.dart (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/cache.dart:153:
_class_dispatcher_matcher.hasMatch(id);
Checking 8 regexps will be slower than checking 1 regexp that uses OR
internally.  What do you think of making a combined regexp?

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:448: return Reused;
What does Reused mean?  The same exact code at the same address with diff
timestamp?

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

https://codereview.chromium.org/197803004/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1284: }
Might be good to make a combo function which parses the full  XXXXX-YYYYY.  Your
call.

[Cutch, 2014-03-13 20:52:26]

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/code_view.html (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/code_view.html:29: }
On 2014/03/13 18:09:09, turnidge wrote:
> To keep in sync with my changes I'm making now...
> 
> Rename .memberBold -> .memberHeader
> 
> Combine rules for .member and .memberHeader since they are currently the same:
> 
> .member, .memberHeader { ...
> 

Done.

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/cache.dart (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/cache.dart:153:
_class_dispatcher_matcher.hasMatch(id);
On 2014/03/13 18:09:09, turnidge wrote:
> Checking 8 regexps will be slower than checking 1 regexp that uses OR
> internally.  What do you think of making a combined regexp?

Done.

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/197803004/diff/20001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:448: return Reused;
On 2014/03/13 18:09:09, turnidge wrote:
> What does Reused mean?  The same exact code at the same address with diff
> timestamp?

The address is used by a newer piece of code.

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

https://codereview.chromium.org/197803004/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1284: }
On 2014/03/13 18:09:09, turnidge wrote:
> Might be good to make a combo function which parses the full  XXXXX-YYYYY. 
Your
> call.

Done.

[Cutch, 2014-03-13 21:00:54]

Committed patchset #3 manually as r33665 (presubmit successful).