In description:
Note: Chrome should be run with --js-flags='--track_gc_object_stats' for
object tracking.
should be --js-flags='--track_gc_object_stats --noincremental-marking'. With
incremental marking we will not correct stats.
https://codereview.chromium.org/1113233002/diff/20001/src/heap/heap.h
File src/heap/heap.h (right):
https://codereview.chromium.org/1113233002/diff/20001/src/heap/heap.h#newcode1455
src/heap/heap.h:1455: const char* GetObjectTypeName(size_t index);
On 2015/05/05 14:26:16, ssid wrote:
On 2015/05/05 14:17:53, Primiano Tucci wrote:
> Should this be near the other ones you are adding?
the previous function does the same kind of operations. that is why
this
function is added here. if the way in which the type index is handled
changes,
then these functions change together.
Note: I am not sure if this function should be a part of Heap class.
Maybe this
should be in objects.h . Waiting for owner's comment on that.
This place is correct. The "index" has meaning only in Heap because it
is the index in the object-stat table.
https://codereview.chromium.org/1113233002/diff/30005/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/1113233002/diff/30005/include/v8.h#newcode4821
include/v8.h:4821: size_t object_size() { return object_size_; }
You probably also want a notion of sub-type or secondary type. For
example a single code object can have multiple HeapObjectStatistics:
- one with type "CODE_TYPE",
- one with type "CODE_TYPE_<code-kind>",
- one with type "CODE_TYPE_<code-age".
So HeapObjectStatistics can share objects. Sum of object_size() will not
be the correct total as it double counts some objects.
https://codereview.chromium.org/1113233002/diff/30005/include/v8.h#newcode5222
include/v8.h:5222: size_t NumberOfHeapObjectsTypes();
Strictly speaking this is not the number of heap object types. This is
the number of heap object statistics that we track (which is more than
the actual heap object types). Can we make it more explicit in the name
that this is about statistics and not generic heap object types?
https://codereview.chromium.org/1113233002/diff/30005/include/v8.h#newcode5233
include/v8.h:5233: bool GetLastGcObjectStatistics(HeapObjectStatistics*
object_statistics,
GetHeapObjectStatisticsAtLastGC(...) to be consistent with other
GetHeap*Statistics functions.
https://codereview.chromium.org/1113233002/diff/30005/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/1113233002/diff/30005/src/api.cc#newcode6930
src/api.cc:6930: }
Two new-lines between functions. Please run "git cl format" to fix
style.
https://codereview.chromium.org/1113233002/diff/30005/src/api.cc#newcode6935
src/api.cc:6935: return false;
Either use {} or put return statements in the previous line.
https://codereview.chromium.org/1113233002/diff/30005/src/api.cc#newcode6946
src/api.cc:6946: if (object_type == nullptr) {
This case shouldn't be happening. Could you please post here which index
has no type?
https://codereview.chromium.org/1113233002/diff/30005/src/heap/heap.cc
File src/heap/heap.cc (right):
https://codereview.chromium.org/1113233002/diff/30005/src/heap/heap.cc#newcode6445
src/heap/heap.cc:6445: if (index > OBJECT_STATS_COUNT)
index >= OBJECT_STATS_COUNT
https://codereview.chromium.org/1113233002/diff/30005/src/heap/heap.cc#newcode6449
src/heap/heap.cc:6449: #define ADJUST_LAST_TIME_OBJECT_COUNT(name) \
The name "ADJUST_LAST_TIME_OBJECT_COUNT" is confusing here. Maybe
"COMPARE_AND_RETURN_NAME" or "CASE_NAME" ?
https://codereview.chromium.org/1113233002/diff/30005/src/heap/heap.h
File src/heap/heap.h (right):
https://codereview.chromium.org/1113233002/diff/30005/src/heap/heap.h#newcode1020
src/heap/heap.h:1020: size_t object_count_last_gc(size_t type) {
s/type/index/ here and below
https://codereview.chromium.org/1113233002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
