Add basic C++ implementation of CPU profiler.

Add basic C++ implementation of CPU profiler.

Committed: http://code.google.com/p/v8/source/detail?r=4189

[Søren Thygesen Gjesse, 2010-03-19 08:58:58]

LGTM

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!
Uppercase n.

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,
Long line please break.

http://codereview.chromium.org/1079006/diff/3001/4003#newcode69
src/cpu-profiler.cc:69: Address start, unsigned size) {
Please break line to have all arguments on separate lines when they cannot fit.

http://codereview.chromium.org/1079006/diff/3001/4003#newcode83
src/cpu-profiler.cc:83: Address start, unsigned size) {
Ditto.

http://codereview.chromium.org/1079006/diff/3001/4003#newcode116
src/cpu-profiler.cc:116: void
ProfilerEventsProcessor::FunctionCreateEvent(Address alias, Address start) {
Long line.

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 {
struct -> class?

http://codereview.chromium.org/1079006/diff/3001/4004#newcode65
src/cpu-profiler.h:65: INLINE(void ModifyCodeMap(CodeMap* code_map)) {
ModifyCodeMap -> UpdateCodeMap?

http://codereview.chromium.org/1079006/diff/3001/4004#newcode103
src/cpu-profiler.h:103: struct TickSampleEventRecord {
struct -> class?

http://codereview.chromium.org/1079006/diff/3001/4004#newcode117
src/cpu-profiler.h:117: class ProfilerEventsProcessor : public Thread {
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)).

http://codereview.chromium.org/1079006/diff/3001/4004#newcode124
src/cpu-profiler.h:124: 
Comment that these methods are used to add events (from the V8 thread).

http://codereview.chromium.org/1079006/diff/3001/4004#newcode152
src/cpu-profiler.h:152: 
Comment that these methods are used to process events pulled from the queues.

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() {
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.

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) {
{}'s on same line when body is empty?

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.
Maybe elaborate a bit here showing how TickSample will be "expanded" into the
TickSampleEventRecord.

[mnaganov (inactive), 2010-03-19 09:33:13]

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.