CpuProfiler: eliminate 2 layers of 4 for CodeCreateEvent calls.

CpuProfiler: eliminate 2 layers of 4 for CodeCreateEvent calls.

The bodies of methods in ProfilerEventProcessor were moved into CpuProfiler.
Multiple NewCodeEntry methods in CpuProfilesCollection were replaced with one which
simply passes arguments to the CodeEntry constructor.
And CpuProfiler just calls this method when it needs a CodeEntry object.

This NewCodeEntry method is required because CpuProfilesCollection keeps ownership of CodeEntry objects.

BUG=255392
TEST=existing tests
R=yangguo@chromium.org, yurys@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15405

Committed: https://code.google.com/p/v8/source/detail?r=15407

[yurys, 2013-06-28 11:33:43]

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler-inl.h
File src/cpu-profiler-inl.h (right):

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler-inl.h#new...
src/cpu-profiler-inl.h:78: void
ProfilerEventsProcessor::Enqueue(CodeEventsContainer* event) {
The argument should be "const CodeEventsContainer&". You could set order on the
enqueued entry only to avoid modifying const object.

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.cc#newcod...
src/cpu-profiler.cc:230: const char* comment) {
comment -> name?

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h
File src/cpu-profiler.h (right):

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h#newcode151
src/cpu-profiler.h:151: INLINE(bool running()) { return running_; }
I don't see much sense in marking these methods inline or always inlined. Let
the compiler decide what is optimal.

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h#newcode251
src/cpu-profiler.h:251: INLINE(static bool
FilterOutCodeCreateEvent(Logger::LogEventsAndTags tag));
I'd rather make it  a static function in cpu-profiler.cc

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.cc
File src/profile-generator.cc (right):

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.cc#n...
src/profile-generator.cc:837: profiles->NewCodeEntry(Logger::FUNCTION_TAG,
Revert?

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:324: const char* GetFunctionName(const char* name) {
Can we keep these methods private and do the econversion in the NewCodeEntry
method?

Also it seems that we can get rid of GetGunctionName and leave simply GetName.
This can be done in a separate CL though.

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:332: INLINE(CodeEntry* NewCodeEntry(
Any reasons to make it inline?

https://codereview.chromium.org/18053004/diff/2001/test/cctest/test-cpu-profi...
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/18053004/diff/2001/test/cctest/test-cpu-profi...
test/cctest/test-cpu-profiler.cc:211: frame1_code->address() + 0x10);
frame1_code->address() + 0x10 -> (frame1_code->instruction_start() +
frame1_code->instruction_end()) / 2? Otherwise, it is unclear what these magic
numbers mean.

[loislo, 2013-06-28 12:33:12]

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.cc#newcod...
src/cpu-profiler.cc:230: const char* comment) {
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> comment -> name?

Done.

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h
File src/cpu-profiler.h (right):

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h#newcode151
src/cpu-profiler.h:151: INLINE(bool running()) { return running_; }
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> I don't see much sense in marking these methods inline or always inlined. Let
> the compiler decide what is optimal.

Done.

https://codereview.chromium.org/18053004/diff/2001/src/cpu-profiler.h#newcode251
src/cpu-profiler.h:251: INLINE(static bool
FilterOutCodeCreateEvent(Logger::LogEventsAndTags tag));
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> I'd rather make it  a static function in cpu-profiler.cc

Done.

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.cc
File src/profile-generator.cc (right):

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.cc#n...
src/profile-generator.cc:837: profiles->NewCodeEntry(Logger::FUNCTION_TAG,
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> Revert?

Done.

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:324: const char* GetFunctionName(const char* name) {
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> Can we keep these methods private and do the econversion in the NewCodeEntry
> method?
> 
> Also it seems that we can get rid of GetGunctionName and leave simply GetName.
> This can be done in a separate CL though.

Will do that in a separate patch

https://codereview.chromium.org/18053004/diff/2001/src/profile-generator.h#ne...
src/profile-generator.h:332: INLINE(CodeEntry* NewCodeEntry(
On 2013/06/28 11:33:43, Yury Semikhatsky wrote:
> Any reasons to make it inline?

Done.

[yurys, 2013-06-28 12:53:43]

lgtm

https://codereview.chromium.org/18053004/diff/8002/src/profile-generator-inl.h
File src/profile-generator-inl.h (right):

https://codereview.chromium.org/18053004/diff/8002/src/profile-generator-inl....
src/profile-generator-inl.h:99: 
Revert?

https://codereview.chromium.org/18053004/diff/8002/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/18053004/diff/8002/src/profile-generator.h#ne...
src/profile-generator.h:334: Name* name,
Remove this function as it is not implemented and not used.

https://codereview.chromium.org/18053004/diff/8002/test/cctest/test-cpu-profi...
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/18053004/diff/8002/test/cctest/test-cpu-profi...
test/cctest/test-cpu-profiler.cc:192: i::Isolate* isolate =
i::Isolate::Current();
env->GetIsolate()

https://codereview.chromium.org/18053004/diff/8002/test/cctest/test-cpu-profi...
test/cctest/test-cpu-profiler.cc:264: i::Isolate* isolate =
i::Isolate::Current();
env->GetIsolate()

[loislo, 2013-06-28 17:02:10]

On 2013/06/28 12:53:43, Yury Semikhatsky wrote:
> lgtm
> 

comments addressed

[Yang, 2013-07-01 09:36:11]

On 2013/06/28 17:02:10, loislo wrote:
> On 2013/06/28 12:53:43, Yury Semikhatsky wrote:
> > lgtm
> > 
> 
> comments addressed

lgtm

[loislo, 2013-07-01 09:39:27]

Committed patchset #5 manually as r15405 (presubmit successful).

[loislo, 2013-07-01 10:12:14]

Committed patchset #6 manually as r15407 (presubmit successful).