Fix CPU profiling for Crankshaft.

Fix CPU profiling for Crankshaft.

The main issue was due to multiple recompilations of functions.  Now
code objects are grouped by function using SFI object address.
JSFunction objects are no longer tracked, instead we track SFI object
moves. To pick a correct code version, we now sample return addresses
instead of JSFunction addresses.

tools/{linux|mac|windows}-tickprocessor scripts differentiate
between code optimization states for the same function
(using * and ~ prefixes introduced earlier).

DevTools CPU profiler treats all variants of function code as
a single function.

ll_prof treats each optimized variant as a separate entry, because
it can disassemble each one of them.

tickprocessor.py not updated -- it is deprecated and will be removed.

BUG=v8/1087,b/3178160
TEST=all existing tests pass, including Chromium layout tests

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

[Vitaly Repeshko, 2011-02-22 15:11:19]

LGTM.

I'm not a huge fan of the "SFI" abbreviation. Can we at least make it local to
the profiler internals (where, BTW, we're already inconsistent: "shared_id" vs.
"sfi_address") and have "SharedFunctionInfoMoveEvent" in the interface?

Also I didn't find the place where "sample->function" is used to find the
calling function. Does it just work automagically?

http://codereview.chromium.org/6551011/diff/1/src/compiler.h
File src/compiler.h (right):

http://codereview.chromium.org/6551011/diff/1/src/compiler.h#newcode269
src/compiler.h:269: Handle<SharedFunctionInfo> shared);
Can we use info->shared_info() instead of passing another shared function info?
If not, please document here why.

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode358
src/cpu-profiler.cc:358: SharedFunctionInfo *shared,
nit: Move *.

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode373
src/cpu-profiler.cc:373: SharedFunctionInfo *shared,
Same nit.

http://codereview.chromium.org/6551011/diff/1/src/handles.cc
File src/handles.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/handles.cc#newcode869
src/handles.cc:869: bool result = CompileLazyHelper(&info, KEEP_EXCEPTION);
Temporary "result" variable is now unnecessary.

http://codereview.chromium.org/6551011/diff/1/src/log.cc
File src/log.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode164
src/log.cc:164: sample->function = Memory::Address_at(sample->sp);
We should rename the "function" field in TickSample. How about "tos_value"?

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode762
src/log.cc:762: if (code == Builtins::builtin(Builtins::LazyCompile)) return;
Can we make the caller do this check for us? (I think it should only matter for
calls made from compiler.cc.)

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode785
src/log.cc:785: SharedFunctionInfo *shared,
Same nit.

http://codereview.chromium.org/6551011/diff/1/src/log.h
File src/log.h (right):

http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode210
src/log.h:210: SharedFunctionInfo *shared,
nit: Move *.

http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode214
src/log.h:214: SharedFunctionInfo *shared,
Same nit.

http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h
File src/profile-generator.h (right):

http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h#newcode273
src/profile-generator.h:273: static const CodeEntry* kSfiCodeEntry;
Add one more "const" to make it really constant.

http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py
File tools/ll_prof.py (right):

http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py#newcode403
tools/ll_prof.py:403: if match.group(6):
Could you please assign these groups (1, 4, 6) to locals (like it's done for
"start_address") to make this code readable?

[mnaganov (inactive), 2011-02-22 16:18:22]

Thanks!

http://codereview.chromium.org/6551011/diff/1/src/compiler.h
File src/compiler.h (right):

http://codereview.chromium.org/6551011/diff/1/src/compiler.h#newcode269
src/compiler.h:269: Handle<SharedFunctionInfo> shared);
On 2011/02/22 15:11:19, Vitaly wrote:
> Can we use info->shared_info() instead of passing another shared function
info?
> If not, please document here why.

For the case when CompilationInfo is built up using Script object, shared_info
isn't available from it. See Compiler::BuildFunctionInfo for an example. I've
documented it.

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode358
src/cpu-profiler.cc:358: SharedFunctionInfo *shared,
On 2011/02/22 15:11:19, Vitaly wrote:
> nit: Move *.

Done.

http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode373
src/cpu-profiler.cc:373: SharedFunctionInfo *shared,
On 2011/02/22 15:11:19, Vitaly wrote:
> Same nit.

Done.

http://codereview.chromium.org/6551011/diff/1/src/handles.cc
File src/handles.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/handles.cc#newcode869
src/handles.cc:869: bool result = CompileLazyHelper(&info, KEEP_EXCEPTION);
On 2011/02/22 15:11:19, Vitaly wrote:
> Temporary "result" variable is now unnecessary.

Done.

http://codereview.chromium.org/6551011/diff/1/src/log.cc
File src/log.cc (right):

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode164
src/log.cc:164: sample->function = Memory::Address_at(sample->sp);
On 2011/02/22 15:11:19, Vitaly wrote:
> We should rename the "function" field in TickSample. How about "tos_value"?

Renamed it to "tos" -- it's shorter.

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode762
src/log.cc:762: if (code == Builtins::builtin(Builtins::LazyCompile)) return;
On 2011/02/22 15:11:19, Vitaly wrote:
> Can we make the caller do this check for us? (I think it should only matter
for
> calls made from compiler.cc.)

Good idea. BTW, I forgot to add a similar check to cpu-profiler, so your
suggestion eliminates an issue ;)

LazyCompile builtins can only be passed from Compiler::RecordFunctionCompilation
and Logger::LogCompiledFunctions, so I moved checks there.

http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode785
src/log.cc:785: SharedFunctionInfo *shared,
On 2011/02/22 15:11:19, Vitaly wrote:
> Same nit.

Done.

http://codereview.chromium.org/6551011/diff/1/src/log.h
File src/log.h (right):

http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode210
src/log.h:210: SharedFunctionInfo *shared,
On 2011/02/22 15:11:19, Vitaly wrote:
> nit: Move *.

Done.

http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode214
src/log.h:214: SharedFunctionInfo *shared,
On 2011/02/22 15:11:19, Vitaly wrote:
> Same nit.

Done.

http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h
File src/profile-generator.h (right):

http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h#newcode273
src/profile-generator.h:273: static const CodeEntry* kSfiCodeEntry;
On 2011/02/22 15:11:19, Vitaly wrote:
> Add one more "const" to make it really constant.

Ah, yes, stupid syntax. I've changed it to "static CodeEntry* const", because I
really don't care about destination constness.

http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py
File tools/ll_prof.py (right):

http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py#newcode403
tools/ll_prof.py:403: if match.group(6):
On 2011/02/22 15:11:19, Vitaly wrote:
> Could you please assign these groups (1, 4, 6) to locals (like it's done for
> "start_address") to make this code readable?

Done.

[mnaganov (inactive), 2011-02-22 16:54:32]

On 2011/02/22 15:11:19, Vitaly wrote:
> LGTM.
> 
> I'm not a huge fan of the "SFI" abbreviation. Can we at least make it local to
> the profiler internals (where, BTW, we're already inconsistent: "shared_id"
vs.
> "sfi_address") and have "SharedFunctionInfoMoveEvent" in the interface?
>

Oh, sorry. I forgot to address this comment. Yes, I will make names more
aligned.
 
> Also I didn't find the place where "sample->function" is used to find the
> calling function. Does it just work automagically?
> 
> http://codereview.chromium.org/6551011/diff/1/src/compiler.h
> File src/compiler.h (right):
> 
> http://codereview.chromium.org/6551011/diff/1/src/compiler.h#newcode269
> src/compiler.h:269: Handle<SharedFunctionInfo> shared);
> Can we use info->shared_info() instead of passing another shared function
info?
> If not, please document here why.
> 
> http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc
> File src/cpu-profiler.cc (right):
> 
> http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode358
> src/cpu-profiler.cc:358: SharedFunctionInfo *shared,
> nit: Move *.
> 
> http://codereview.chromium.org/6551011/diff/1/src/cpu-profiler.cc#newcode373
> src/cpu-profiler.cc:373: SharedFunctionInfo *shared,
> Same nit.
> 
> http://codereview.chromium.org/6551011/diff/1/src/handles.cc
> File src/handles.cc (right):
> 
> http://codereview.chromium.org/6551011/diff/1/src/handles.cc#newcode869
> src/handles.cc:869: bool result = CompileLazyHelper(&info, KEEP_EXCEPTION);
> Temporary "result" variable is now unnecessary.
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.cc
> File src/log.cc (right):
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode164
> src/log.cc:164: sample->function = Memory::Address_at(sample->sp);
> We should rename the "function" field in TickSample. How about "tos_value"?
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode762
> src/log.cc:762: if (code == Builtins::builtin(Builtins::LazyCompile)) return;
> Can we make the caller do this check for us? (I think it should only matter
for
> calls made from compiler.cc.)
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.cc#newcode785
> src/log.cc:785: SharedFunctionInfo *shared,
> Same nit.
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.h
> File src/log.h (right):
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode210
> src/log.h:210: SharedFunctionInfo *shared,
> nit: Move *.
> 
> http://codereview.chromium.org/6551011/diff/1/src/log.h#newcode214
> src/log.h:214: SharedFunctionInfo *shared,
> Same nit.
> 
> http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h
> File src/profile-generator.h (right):
> 
>
http://codereview.chromium.org/6551011/diff/1/src/profile-generator.h#newcode273
> src/profile-generator.h:273: static const CodeEntry* kSfiCodeEntry;
> Add one more "const" to make it really constant.
> 
> http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py
> File tools/ll_prof.py (right):
> 
> http://codereview.chromium.org/6551011/diff/1/tools/ll_prof.py#newcode403
> tools/ll_prof.py:403: if match.group(6):
> Could you please assign these groups (1, 4, 6) to locals (like it's done for
> "start_address") to make this code readable?