Just a few nits. Some of them not even on code that you touched, sorry for
that.
:)
Also, I think it's time for a second-order macro of all spaces present in
the V8
heap. Yang suggested a similar thing in response to the previous CL
introducing
the counters. But you don't need to do that as part of this CL, I can do
that
after we landed this one.
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.cc
File src/counters.cc (right):
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.cc#newcode1
src/counters.cc:1: // Copyright 2007-2008 the V8 project authors. All
rights reserved.
2012
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.cc#newcode68
src/counters.cc:68: if (Enabled())
Curly brackets if not on one line.
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.h
File src/counters.h (right):
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.h#newcode1
src/counters.h:1: // Copyright 2007-2008 the V8 project authors. All
rights reserved.
2012
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.h#newcode173
src/counters.h:173: return ptr_;
Aww, that should be on one line.
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.h#newcode202
src/counters.h:202: // Similar to StatsCounter, the class Histogram
represents a dynamically
Can we remove the reference to StatsCounter here? Just "A Histogram
represents [...]"
https://chromiumcodereview.appspot.com/10695056/diff/1/src/counters.h#newcode220
src/counters.h:220: // Returns false if table is full.
Let's turn that into just one line: "Returns true if this histogram is
enabled."
https://chromiumcodereview.appspot.com/10695056/diff/1/src/heap.cc
File src/heap.cc (right):
https://chromiumcodereview.appspot.com/10695056/diff/1/src/heap.cc#newcode449
src/heap.cc:449: static_cast<int>(new_space()->Available()) / 1024);
Better use the "KB" macro here. And on the following lines.
https://chromiumcodereview.appspot.com/10695056/diff/1/src/v8-counters.cc
File src/v8-counters.cc (right):
https://chromiumcodereview.appspot.com/10695056/diff/1/src/v8-counters.cc#newcode1
src/v8-counters.cc:1: // Copyright 2007-2008 the V8 project authors. All
rights reserved.
2012
https://chromiumcodereview.appspot.com/10695056/diff/1/src/v8-counters.h
File src/v8-counters.h (right):
https://chromiumcodereview.appspot.com/10695056/diff/1/src/v8-counters.h#newcode1
src/v8-counters.h:1: // Copyright 2011 the V8 project authors. All
rights reserved.
2012
https://chromiumcodereview.appspot.com/10695056/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
