Thank you very much!
http://codereview.chromium.org/1079006/diff/3001/4002
File src/cpu-profiler-inl.h (right):
http://codereview.chromium.org/1079006/diff/3001/4002#newcode43
src/cpu-profiler-inl.h:43: evt->order = enqueue_order_; // no
increment!
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Uppercase n.
Done.
http://codereview.chromium.org/1079006/diff/3001/4003
File src/cpu-profiler.cc (right):
http://codereview.chromium.org/1079006/diff/3001/4003#newcode54
src/cpu-profiler.cc:54: String* resource_name, int line_number,
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Long line please break.
Fixed in the next patch set.
http://codereview.chromium.org/1079006/diff/3001/4003#newcode69
src/cpu-profiler.cc:69: Address start, unsigned size) {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Please break line to have all arguments on separate lines when they
cannot fit.
Done.
http://codereview.chromium.org/1079006/diff/3001/4003#newcode83
src/cpu-profiler.cc:83: Address start, unsigned size) {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Ditto.
Done.
http://codereview.chromium.org/1079006/diff/3001/4003#newcode116
src/cpu-profiler.cc:116: void
ProfilerEventsProcessor::FunctionCreateEvent(Address alias, Address
start) {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Long line.
Fixed in the next patch set.
http://codereview.chromium.org/1079006/diff/3001/4004
File src/cpu-profiler.h (right):
http://codereview.chromium.org/1079006/diff/3001/4004#newcode45
src/cpu-profiler.h:45: struct CodeEventRecord {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
struct -> class?
Done.
http://codereview.chromium.org/1079006/diff/3001/4004#newcode65
src/cpu-profiler.h:65: INLINE(void ModifyCodeMap(CodeMap* code_map)) {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
ModifyCodeMap -> UpdateCodeMap?
Renamed here and in below.
http://codereview.chromium.org/1079006/diff/3001/4004#newcode103
src/cpu-profiler.h:103: struct TickSampleEventRecord {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
struct -> class?
Done.
http://codereview.chromium.org/1079006/diff/3001/4004#newcode117
src/cpu-profiler.h:117: class ProfilerEventsProcessor : public Thread {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
I think you should add a comment about this class implements both the
profile
events processor thread and the methods called by the events producer,
that is
the public part of this class is called by the V8 code (is the V8
thread(s)).
Done.
http://codereview.chromium.org/1079006/diff/3001/4004#newcode124
src/cpu-profiler.h:124:
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Comment that these methods are used to add events (from the V8
thread).
Done.
http://codereview.chromium.org/1079006/diff/3001/4004#newcode152
src/cpu-profiler.h:152:
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Comment that these methods are used to process events pulled from the
queues.
Done.
http://codereview.chromium.org/1079006/diff/5001/6002
File src/cpu-profiler-inl.h (right):
http://codereview.chromium.org/1079006/diff/5001/6002#newcode40
src/cpu-profiler-inl.h:40: TickSample*
ProfilerEventsProcessor::TickSampleEvent() {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Could you please make a comment here explaining how
TickSampleEventRecords are
stored directly in the circular buffer, and that this returns a
pointer to the
next free entry to write to.
Added a comment in the class definition (.h file.)
http://codereview.chromium.org/1079006/diff/5001/6003
File src/cpu-profiler.cc (right):
http://codereview.chromium.org/1079006/diff/5001/6003#newcode48
src/cpu-profiler.cc:48: enqueue_order_(0) {
On 2010/03/19 08:58:59, Søren Gjesse wrote:
{}'s on same line when body is empty?
Done.
http://codereview.chromium.org/1079006/diff/5001/6004
File src/cpu-profiler.h (right):
http://codereview.chromium.org/1079006/diff/5001/6004#newcode105
src/cpu-profiler.h:105: // SamplingCircularQueue::kClear, while
sample.pc can't.
On 2010/03/19 08:58:59, Søren Gjesse wrote:
Maybe elaborate a bit here showing how TickSample will be "expanded"
into the
TickSampleEventRecord.
Done.
http://codereview.chromium.org/1079006
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply
to this email with the words "REMOVE ME" as the subject.
