Provide a mechanism for dumping of heap profiles on predefined events.

Provide a mechanism for dumping of heap profiles on predefined events.

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

[siva, 2012-12-07 00:33:02]

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

https://codereview.chromium.org/11478012/diff/1/runtime/vm/exceptions.cc#newc...
runtime/vm/exceptions.cc:179: if (incoming_exception.Equals(out_of_memory)) {
The handle and if check seems to be repeated twice here?

https://codereview.chromium.org/11478012/diff/1/runtime/vm/exceptions.cc#newc...
runtime/vm/exceptions.cc:184: isolate->heap()->ProfileToFile(filename);
Why not change the signature of ProfileToFile to be
void ProfileToFile(const char* suffix_name) const;

That way you could move all the filename computation code into ProfileToFile and
not have have to repeat it in two places.

e.g:

isolate->heap()->ProfileToFile("isolate-initialize");
isolate->heap()->ProfileToFile("isolate-out-of-memory");

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

https://codereview.chromium.org/11478012/diff/1/runtime/vm/isolate.cc#newcode118
runtime/vm/isolate.cc:118: if (result.IsInstance() &&
FLAG_heap_profile_out_of_memory) {
This check seems incorrect, at this point result will
be an UnhandledException if an OOM was encountered while handling the message.

You can then do something like:
const Instance& exception = Instance::Handle(result.exception());
if (exception.raw() == isolate->object_store()->out_of_memory()) {
   ....
}

[cshapiro, 2012-12-08 07:29:45]

Sorry about the noise but I got this error

The following errors occured:
Base files missing.
Try to upload the changeset again.

rebased and re-uploaded thinking the original upload did not happen

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

https://codereview.chromium.org/11478012/diff/1/runtime/vm/exceptions.cc#newc...
runtime/vm/exceptions.cc:179: if (incoming_exception.Equals(out_of_memory)) {
Sorry about that.  I actually caught this earlier.  Removed.

https://codereview.chromium.org/11478012/diff/1/runtime/vm/exceptions.cc#newc...
runtime/vm/exceptions.cc:184: isolate->heap()->ProfileToFile(filename);
Good idea.  Done.  (Saves ~3 places, actually.)

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

https://codereview.chromium.org/11478012/diff/1/runtime/vm/isolate.cc#newcode118
runtime/vm/isolate.cc:118: if (result.IsInstance() &&
FLAG_heap_profile_out_of_memory) {
I am really confused by this.  At this point result can be an Error or
UnhandledException but the handle type is Object.

I think what you are suggesting is that if an OOM is thrown, it will be wrapped
in an UnhandledException.  However, since any Error is admitted, it seems
possible for an OOM to be passed as an argument as well.

What is doubly confusing is that UnhandledException is a subclass of Error so
the result.IsUnhandledException() test seems pointless.

Anyway, I have handled both cases explicitly.  If I have got it wrong, I can
easily prune either case.

[siva, 2012-12-10 05:06:36]

LGTM

https://codereview.chromium.org/11478012/diff/7001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/11478012/diff/7001/runtime/vm/isolate.cc#newc...
runtime/vm/isolate.cc:149: exception ^= result.raw();
I think this else part is not needed and the OOM check can be done under
if (result.IsUnhandledException()) {
  ....
   ...
} itself.