https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8-profiler.h File include/v8-profiler.h (right):
https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8-profiler.h#newcode420 include/v8-profiler.h:420: static void StartHeapObjectsTracking(); Comments are required. https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8-profiler.h#newcode422 include/v8-profiler.h:422: static void PushHeapObjectsStats(OutputStream* stream); Why "Push"? Maybe "Serialize"? https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8.h File include/v8.h (right): https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8.h#newcode3745 include/v8.h:3745: kUint32 = 1 Any particular reason for not sending this data as ASCII / JSON? This is just a stream of numbers, AFAICT. https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8.h#newcode3764 include/v8.h:3764: nit: no blank line https://chromiumcodereview.appspot.com/10049002/diff/4001/include/v8.h#newcode3766 include/v8.h:3766: * Writes the next chunk of snapshot data into the stream. Writing Does it really write snapshot data? https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.cc File src/profile-generator.cc (right): https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.cc#newcode1409 src/profile-generator.cc:1409: SnapshotObjectId HeapObjectsMap::FindOrAddEntry(Address addr) { This functions seems to reuse code from FindEntry, FindObject and AddEntry. Can you please try to avoid code duplication? https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.cc#newcode1455 src/profile-generator.cc:1455: int collected_fragment_updates = 0; This variable seems to be write-only. https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.h File src/profile-generator.h (right): https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.h#newcode758 src/profile-generator.h:758: struct FragmentInfo { Please move under EntryInfo definition. https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.h#newcode760 src/profile-generator.h:760: SnapshotObjectId id; 'id' isn't very descriptive. Should it be something like 'last_id'? https://chromiumcodereview.appspot.com/10049002/diff/4001/src/profile-generator.h#newcode778 src/profile-generator.h:778: void StartHeapObjectsTracking() { I think, non-trivial methods should be implemented in the .cc file. https://chromiumcodereview.appspot.com/10049002/diff/4001/test/cctest/test-heap-profiler.cc File test/cctest/test-heap-profiler.cc (right): https://chromiumcodereview.appspot.com/10049002/diff/4001/test/cctest/test-heap-profiler.cc#newcode749 test/cctest/test-heap-profiler.cc:749: v8::Persistent<v8::String> p_string = This handle can be local, you have a HandleScope at the function level, so the string will be kept alive until you return. https://chromiumcodereview.appspot.com/10049002/diff/4001/test/cctest/test-heap-profiler.cc#newcode766 test/cctest/test-heap-profiler.cc:766: I think, you should also test stats update on object deletion. https://chromiumcodereview.appspot.com/10049002/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
