https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc
File src/profile-generator.cc (right):
https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc#newcode2801
src/profile-generator.cc:2801: "native",
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
Does this look descriptive enough? If implicit refs are only for event
handlers,
perhaps we can call this more specific?
For now it seems good enough. We cannot assume that all implicit
references on all embedders will always be for event handlers only even
if it is so in Chromium at the moment. We would like to extend
information about such references with e.g. event names and probably
something else but for now just having a strong reference reflected in
the heap snapshot is enough.
https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc#newcode2825
src/profile-generator.cc:2825: if (EstimateObjectsCount() <= 0) {
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
Let's rewrite this as follows to avoid cleanup code duplication:
if (EstimateObjectsCount() > 0) {
for(...) {
}
SetRootNative...
}
filler_ = NULL;
return true;
Done.
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc
File test/cctest/test-heap-profiler.cc (right):
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode813
test/cctest/test-heap-profiler.cc:813: for (int i = 0; i < 4; i++)
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
nit: brackets
Done. Are they required by V8 coding style?
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode832
test/cctest/test-heap-profiler.cc:832:
v8::Persistent<v8::Object>::Cast(objects_[1]), &objects_[2], 2);
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
Have you forgot to add 1->3 reference, or what does this comment mean?
No, the comment is perfectly fine, although Ilya asked the same
question. Note that in the second call we pass array of length 2.
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode835
test/cctest/test-heap-profiler.cc:835: v8::Persistent<v8::Value>
objects_[4];
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
I think, using an enum for test objects and their count will be more
readable
and robust.
Well, I started with storing each object in its own field but it turned
out that the only thing we need of them is their ordinal number. What
kind of enum constants do you suggest? I'd leave this part as is.
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode854
test/cctest/test-heap-profiler.cc:854: const v8::HeapGraphNode* o0 =
GetProperty(
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote:
This name looks weird. In other tests names like "obj0" are used.
Done.
https://chromiumcodereview.appspot.com/9316092/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
