LGTM with comments.
How much memory is taken up by a heap snapshot? I assume that your
StringStorage
is to avoid keeping a copy of all stings in each snapshot. How is the ratoi
between the string data and the rest?
http://codereview.chromium.org/2722005/diff/5001/6002
File src/profile-generator.cc (right):
http://codereview.chromium.org/2722005/diff/5001/6002#newcode876
src/profile-generator.cc:876: int HeapEntry::TotalSize() {
Please use constant instead of -1.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode881
src/profile-generator.cc:881: int HeapEntry::NonSharedTotalSize() {
Ditto.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode919
src/profile-generator.cc:919: void Apply(HeapEntry* entry) {
Please use constants for the different kind of paint. It is not clear
what the difference between color 1 and 2 is.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode931
src/profile-generator.cc:931: int
HeapEntry::CalculateNonSharedTotalSize() {
Please add some comments to indicate what the two loops does.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode965
src/profile-generator.cc:965: NonSharedSizeCalculator calculator;
Looks as if the NonSharedSizeCalculator only looks at paint == 1. What
am I missing here?
http://codereview.chromium.org/2722005/diff/5001/6002#newcode1002
src/profile-generator.cc:1002: };
Please add another empty line.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode1112
src/profile-generator.cc:1112: case JS_OBJECT: return "/js obj/";
Why not just "object" instead of "js obj"?
http://codereview.chromium.org/2722005/diff/5001/6002#newcode1233
src/profile-generator.cc:1233: next_gc_root_id_(1) {
next_gc_root_id_ is not used.
http://codereview.chromium.org/2722005/diff/5001/6002#newcode1448
src/profile-generator.cc:1448: HeapSnapshot* snapshot = new
HeapSnapshot(this, name, uid);
It is by design, that HeapSnapshotGenerator is nou used immediately to
fill the snapshot?
http://codereview.chromium.org/2722005/diff/5001/6002#newcode1495
src/profile-generator.cc:1495: ExtractElementReferences(js_obj, entry);
How about the references to the prototype and constructor?
http://codereview.chromium.org/2722005/diff/5001/6003
File src/profile-generator.h (right):
http://codereview.chromium.org/2722005/diff/5001/6003#newcode609
src/profile-generator.h:609:
Please add a few lines describing these classes and how they connect. I
have guessed that you create a SnapshotCollection, call NewSnapshot on
that and use HeapSnapshotGenerator afterwards, sounds right?
http://codereview.chromium.org/2722005/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
