https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.cc
File src/heap-profiler.cc (right):

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.cc#newcode162
src/heap-profiler.cc:162: DropCompiledCode();
Would it make sense to run GC and collect dropped code objects before
updating the map?

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.cc#newcode182
src/heap-profiler.cc:182: Heap* heap_ = heap();
style: heap_ -> heap or use heap() directly, without assigning to a
local variable.

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.cc#newcode193
src/heap-profiler.cc:193:
Handle<Code>(isolate->builtins()->builtin(Builtins::kLazyCompile));
style: 4 spaces indent

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.cc#newcode208
src/heap-profiler.cc:208: if (!shared->script()->IsScript()) continue;
We will need to do something with the precompiled functions that have no
script, otherwise we will miss their allocations but I think it's fine
for the first iteration.

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.h
File src/heap-profiler.h (right):

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.h#newcode106
src/heap-profiler.h:106: private:
style: blank line above "private:"

https://codereview.chromium.org/22852024/diff/40001/src/heap-profiler.h#newcode109
src/heap-profiler.h:109: void UpdateMap() { snapshots_->UpdateMap(); }
Just use snapshots_->UpdateMap() instead.

https://codereview.chromium.org/22852024/diff/40001/src/heap-snapshot-generator.h
File src/heap-snapshot-generator.h (right):

https://codereview.chromium.org/22852024/diff/40001/src/heap-snapshot-generator.h#newcode256
src/heap-snapshot-generator.h:256: void UpdateMap();
There is already UpdateHeapObjectsMap which does almost the same, I
believe we can reuse it.

https://codereview.chromium.org/22852024/diff/40001/src/spaces.h
File src/spaces.h (right):

https://codereview.chromium.org/22852024/diff/40001/src/spaces.h#newcode1909
src/spaces.h:1909: inline HeapObject* AllocateRawHelper(int
size_in_bytes);
What's the point in the inline modifier here?

https://codereview.chromium.org/22852024/diff/40001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/22852024/diff/40001/src/x64/code-stubs-x64.cc#newcode4945
src/x64/code-stubs-x64.cc:4945: // Make a long jump due to
RecordObjectAllocation inside
You could calculate

Label::Distance distance =
masm->isolate()->heap_profiler()->is_tracking_allocations() ?
Label::kFar : Label::kNear;

once and use it in the two jumps below.

https://codereview.chromium.org/22852024/diff/40001/test/cctest/cctest.h
File test/cctest/cctest.h (left):

https://codereview.chromium.org/22852024/diff/40001/test/cctest/cctest.h#oldcode81
test/cctest/cctest.h:81:
revert this

https://codereview.chromium.org/22852024/diff/40001/test/cctest/cctest.h
File test/cctest/cctest.h (right):

https://codereview.chromium.org/22852024/diff/40001/test/cctest/cctest.h#newcode357
test/cctest/cctest.h:357: virtual ~HeapObjectsTracker() {
No need to make it virtual.

https://codereview.chromium.org/22852024/diff/40001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (left):

https://codereview.chromium.org/22852024/diff/40001/test/cctest/test-heap.cc#oldcode3255
test/cctest/test-heap.cc:3255:
Revert this.

https://codereview.chromium.org/22852024/

--
--
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/groups/opt_out.

Reply via email to