https://codereview.chromium.org/22852024/diff/84001/include/v8-profiler.h
File include/v8-profiler.h (right):
https://codereview.chromium.org/22852024/diff/84001/include/v8-profiler.h#newcode478
include/v8-profiler.h:478: * and tracking of heap objects population
statistics.
On 2013/10/02 18:00:29, Hannes Payer wrote:
Please fill up the 80 characters line width.
Done.
https://codereview.chromium.org/22852024/diff/84001/include/v8-profiler.h#newcode485
include/v8-profiler.h:485: * population statistics data.
On 2013/10/02 18:00:29, Hannes Payer wrote:
Please fill up the 80 characters line width.
Done.
https://codereview.chromium.org/22852024/diff/84001/src/heap-snapshot-generator.cc
File src/heap-snapshot-generator.cc (right):
https://codereview.chromium.org/22852024/diff/84001/src/heap-snapshot-generator.cc#newcode447
src/heap-snapshot-generator.cc:447: FindOrAddEntry(addr, size, false);
On 2013/10/02 18:00:29, Hannes Payer wrote:
Why do we have two implementations of the same functions?
Sometimes we want just to update the information (the size) of the
object that has already been tracked. This works pretty much the same as
reqistering a new object, but one more call of NewObjectEvent (and
NewObject) can confuse other developers, so we decided to create
separate methods for it.
For example, here
https://codereview.chromium.org/22852024/diff/84001/src/heap.cc both an
object and its allocation memento are allocated in one go. So then we
track the allocation memento as a new object and adjust information
about the size of the original object itself.
https://codereview.chromium.org/22852024/diff/84001/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/22852024/diff/84001/src/heap.cc#newcode4979
src/heap.cc:4979: HeapProfiler* profiler = isolate()->heap_profiler();
On 2013/10/02 18:20:04, Hannes Payer wrote:
On 2013/10/02 18:00:29, Hannes Payer wrote:
> Why do you track the allocation of the memento separately? And why
don't you
> account the allocation in the if branch before?
Ok, ignore question two, because you allocate using AllocateRaw. But
why do you
track the memento separately?
The memento is treated as a separate HeapObject when the heap is being
iterated, so if we don't track it, we'll have untracked allocations.
https://codereview.chromium.org/22852024/diff/84001/src/spaces-inl.h
File src/spaces-inl.h (right):
https://codereview.chromium.org/22852024/diff/84001/src/spaces-inl.h#newcode293
src/spaces-inl.h:293: } while (false);
On 2013/10/02 18:00:29, Hannes Payer wrote:
I don't like the goto emulation here. Why don't we keep the original
method and
before return object we call
if (event == NEW_OBJECT) profiler->NewObjectEvent(...)
NewObjectEvent could check internally if allocation tracking is turned
on or you
keep the check outside, as you like.
Ok, corrected it keeping the original method.
https://codereview.chromium.org/22852024/diff/84001/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):
https://codereview.chromium.org/22852024/diff/84001/src/x64/macro-assembler-x64.h#newcode1124
src/x64/macro-assembler-x64.h:1124: // Record a JS object allocation if
allocations tracking
On 2013/10/02 18:00:29, Hannes Payer wrote:
Should fit in one line.
Done.
https://codereview.chromium.org/22852024/diff/84001/test/cctest/test-heap-profiler.cc
File test/cctest/test-heap-profiler.cc (right):
https://codereview.chromium.org/22852024/diff/84001/test/cctest/test-heap-profiler.cc#newcode2010
test/cctest/test-heap-profiler.cc:2010: // This is an example of using
checking of JS allocations tracking in a test.
On 2013/10/02 18:00:29, Hannes Payer wrote:
Either remove the test if it is not testing anything or add a proper
test. I
would prefer adding a proper test.
This test checks that we don't miss some simplest allocations and
HeapObjectsTracker works correctly. In the HeapObjectsTracker
constructor, allocations tracking is switched on. In the destructor,
allocations tracking is checked and untracked objects are found (if
any).
https://codereview.chromium.org/22852024/diff/84001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):
https://codereview.chromium.org/22852024/diff/84001/test/cctest/test-heap.cc#newcode3232
test/cctest/test-heap.cc:3232: HeapObjectsTracker tracker;
On 2013/10/02 18:00:29, Hannes Payer wrote:
Why is that here?
I used HeapObjectsTracker here to find some untracked allocations that
occur during this test. In fact, HeapObjectsTracker can be used in all
tests where JS allocations are performed.
I've removed it here now as there's a special test for it.
https://codereview.chromium.org/22852024/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
