Issue 11734028: - Consolidate verbose-gc output to be a single line which can be imported easily into spreadsheets.

Issue 11734028: - Consolidate verbose-gc output to be a single line which can be imported easily into spreadsheets. (Closed)

Created:
7 years, 11 months ago by Ivan Posva

Modified:
7 years, 11 months ago

Reviewers:
siva

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

- Consolidate verbose-gc output to be a single line which can be imported easily into spreadsheets. Committed: https://code.google.com/p/dart/source/detail?r=16624

Patch Set 1 #

Total comments: 8

Created: 7 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+214 lines, -116 lines)			Patch
M	runtime/platform/globals.h	View	1 chunk	+5 lines, -1 line	0 comments	Download
M	runtime/vm/dart_api_impl_test.cc	View	1 chunk	+1 line, -2 lines	0 comments	Download
M	runtime/vm/heap.h	View	2 chunks	+50 lines, -1 line	5 comments	Download
M	runtime/vm/heap.cc	View	6 chunks	+112 lines, -18 lines	0 comments	Download
M	runtime/vm/isolate.h	View	3 chunks	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/isolate.cc	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/pages.h	View	2 chunks	+1 line, -4 lines	0 comments	Download
M	runtime/vm/pages.cc	View	7 chunks	+20 lines, -37 lines	0 comments	Download
M	runtime/vm/scavenger.h	View	2 chunks	+2 lines, -4 lines	0 comments	Download
M	runtime/vm/scavenger.cc	View	11 chunks	+18 lines, -49 lines	3 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

siva

LGTM with one ASSERT comment. https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h File runtime/vm/heap.h (right): https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h#newcode182 runtime/vm/heap.h:182: ASSERT((id > 0) && ...

7 years, 11 months ago (2013-01-04 01:40:12 UTC) #2

Ivan Posva

7 years, 11 months ago (2013-01-04 02:14:31 UTC) #3

https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h
File runtime/vm/heap.h (right):

https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h#newcode182
runtime/vm/heap.h:182: ASSERT((id > 0) && (id < GCStats::kDataEntries));
On 2013/01/04 01:40:12, siva wrote:
> This asserts here that id should be > 0 but I see calls
> like RecordTime(0, ...) in pages.cc
> 
> That should have asserted?

Done.

https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h#newcode187
runtime/vm/heap.h:187: ASSERT((id > 0) && (id < GCStats::kDataEntries));
On 2013/01/04 01:40:12, siva wrote:
> This asserts here that id should be > 0 but I see calls
> like RecordData(0, ...) in pages.cc
> 
> That should have asserted?

Done.

https://codereview.chromium.org/11734028/diff/1/runtime/vm/heap.h#newcode187
runtime/vm/heap.h:187: ASSERT((id > 0) && (id < GCStats::kDataEntries));
On 2013/01/04 01:40:12, siva wrote:
> This asserts here that id should be > 0 but I see calls
> like RecordData(0, ...) in pages.cc
> 
> That should have asserted?

Done.

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

https://codereview.chromium.org/11734028/diff/1/runtime/vm/scavenger.cc#newco...
runtime/vm/scavenger.cc:431: heap_->RecordData(3, duplicates);
On 2013/01/04 01:40:12, siva wrote:
> It might be more readable to have an enumeration on
> top as:
> enum {
>   kStoreBufferEntries = 0,
>   kStoreBufferDuplicates = 1,
>   kStoreBufferBlockEntries = 2,
>   kStoreBufferBlockDuplicates = 3
> };

Done. Here and in pages.cc

Expand Messages | Collapse Messages