Resubmission of native memory allocation info surfacing in Observatory. Fixed crashing tests and st…

Resubmission of native memory allocation info surfacing in Observatory. Fixed crashing tests and stack trace collection skip counts for different configurations.


Fixed missing method referenced when MallocHooks is unsupported.

BUG=
R=johnmccutchan@google.com

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

[bkonyi, 2017-03-24 21:54:09]

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

[siva, 2017-03-24 22:50:59]

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

[siva, 2017-03-24 22:51:00]

DBC

https://codereview.chromium.org/2771293003/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/service.status (right):

https://codereview.chromium.org/2771293003/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/service.status:103:
get_native_allocation_samples_test: RuntimeError # Unsupported.
If it is not supported why not set the status to Skip instead of RuntimeError ?

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

https://codereview.chromium.org/2771293003/diff/20001/runtime/vm/malloc_hooks...
runtime/vm/malloc_hooks.cc:174: #endif
Adding such target #ifs in a common file just makes the code hard to read. Can
this be abstracted into target specific files.

[Cutch, 2017-03-27 16:37:33]

lgtm after Siva's comments

[bkonyi, 2017-03-27 17:50:26]

I've addressed Siva's comments and everything should be good to go.

https://codereview.chromium.org/2771293003/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/service.status (right):

https://codereview.chromium.org/2771293003/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/service.status:103:
get_native_allocation_samples_test: RuntimeError # Unsupported.
On 2017/03/24 22:51:00, siva wrote:
> If it is not supported why not set the status to Skip instead of RuntimeError
?

You're right, that would make more sense. Using RuntimeError would prevent
failures, but no point running tests on unsupported platforms.

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

https://codereview.chromium.org/2771293003/diff/20001/runtime/vm/malloc_hooks...
runtime/vm/malloc_hooks.cc:174: #endif
On 2017/03/24 22:51:00, siva wrote:
> Adding such target #ifs in a common file just makes the code hard to read. Can
> this be abstracted into target specific files.

Done.

[Cutch, 2017-03-27 17:52:29]

still lgtm

https://codereview.chromium.org/2771293003/diff/40001/runtime/vm/malloc_hooks.cc
File runtime/vm/malloc_hooks.cc (right):

https://codereview.chromium.org/2771293003/diff/40001/runtime/vm/malloc_hooks...
runtime/vm/malloc_hooks.cc:438: bool result =
MallocHooksState::address_map()->Remove(ptr);
const bool ...

[bkonyi, 2017-03-27 20:25:40]

Description was changed from

==========
Resubmission of native memory allocation info surfacing in Observatory. Fixed
crashing tests and stack trace collection skip counts for different
configurations.


Fixed missing method referenced when MallocHooks is unsupported.

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

to

==========
Resubmission of native memory allocation info surfacing in Observatory. Fixed
crashing tests and stack trace collection skip counts for different
configurations.


Fixed missing method referenced when MallocHooks is unsupported.

BUG=
R=johnmccutchan@google.com

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

[bkonyi, 2017-03-27 20:25:41]

Committed patchset #6 (id:100001) manually as
7c820f4363bd9a713f7c6455f2d56e640045ec8c (presubmit successful).