Report the reason for a garbage collection when verbose GC is enabled.

Report the reason for a garbage collection when verbose GC is enabled.

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

[siva, 2012-08-27 16:54:49]

lgtm

http://codereview.chromium.org/10872074/diff/1/runtime/vm/heap.cc
File runtime/vm/heap.cc (right):

http://codereview.chromium.org/10872074/diff/1/runtime/vm/heap.cc#newcode191
runtime/vm/heap.cc:191: code_space_->MarkSweep(invoke_api_callbacks, "code
space");
Can we list these reasons in heap.h as static const char* fields. That way we
would have one spot to look for all reasons and people would not put some random
strings (lower case/upper case/missing spaces etc).

e.g:
static const char* new_space_full_ = "new space full";

and use Heap::new_space_full() in the code.

I think the strings could be a little more verbose
e.g "new space exhausted" instead of just "new space"

[Ivan Posva, 2012-08-27 22:04:23]

LGTM with comments.

-Ivan

https://chromiumcodereview.appspot.com/10872074/diff/1/runtime/vm/heap.cc
File runtime/vm/heap.cc (right):

https://chromiumcodereview.appspot.com/10872074/diff/1/runtime/vm/heap.cc#new...
runtime/vm/heap.cc:191: code_space_->MarkSweep(invoke_api_callbacks, "code
space");
On 2012/08/27 16:54:49, asiva wrote:
> Can we list these reasons in heap.h as static const char* fields. That way we
> would have one spot to look for all reasons and people would not put some
random
> strings (lower case/upper case/missing spaces etc).
> 
> e.g:
> static const char* new_space_full_ = "new space full";
> 
> and use Heap::new_space_full() in the code.
> 
> I think the strings could be a little more verbose
> e.g "new space exhausted" instead of just "new space"

I like the suggestion of having the different reasons be collected in a central
place, but I am not sure they need to be more verbose.

https://chromiumcodereview.appspot.com/10872074/diff/1/runtime/vm/scavenger.h
File runtime/vm/scavenger.h (right):

https://chromiumcodereview.appspot.com/10872074/diff/1/runtime/vm/scavenger.h...
runtime/vm/scavenger.h:46: scavenge("debugging");
Lower-case?

[cshapiro, 2012-08-27 22:04:45]

If you want to avoid having random strings we can probably do better...  The
last time I added this kind of logging to a collector I created a table of
reason strings and passed around an enum value which could retrieve the message.
 Kind of like errno and strerror().  What would you prefer?

[Ivan Posva, 2012-08-27 22:08:16]

On 2012/08/27 22:04:45, cshapiro wrote:
> If you want to avoid having random strings we can probably do better...  The
> last time I added this kind of logging to a collector I created a table of
> reason strings and passed around an enum value which could retrieve the
message.
>  Kind of like errno and strerror().  What would you prefer?

enum

-Ivan

[cshapiro, 2012-08-28 03:34:29]

PTAL

I might be reading into the comments too much, but there seems to be some
difference of opinion.  Also, I made an oversight in my design suggestion.  You
cannot plumb an enum from Heap down through to Scavenge and MarkSweep as this
creates a circular dependency.

[Ivan Posva, 2012-08-28 23:44:59]

LGTM, at some point we should consolidate the verbose GC output into its own are
so that all collectors use a unified format.

-Ivan