Propagate bailout and dont_optimize reasons to cpu-profiler

Functions may not be optimized and we would like to know in cpu profiler what was the reason.

Current v8 implementation may disable optimization for a particular function or block it with help of dont_optimize flag.
The patch propagates the reason of that to the SharedFunctionInfo where cpu profiler can get it.

SharedFunctionInfo is a heap object so I extracted 8 bits from OptsCount for handling bailout reason code.


BUG=none
TEST=test-profile-generator/BailoutReason
R=yangguo@chromium.org

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

[yurys, 2013-09-02 14:40:04]

https://codereview.chromium.org/23817003/diff/1/include/v8-profiler.h
File include/v8-profiler.h (right):

https://codereview.chromium.org/23817003/diff/1/include/v8-profiler.h#newcode63
include/v8-profiler.h:63: Handle<String> GetBailoutReason() const;
Can we start returning const char* instead of JS strings?

https://codereview.chromium.org/23817003/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/23817003/diff/1/src/ast.h#newcode2316
src/ast.h:2316: inline BailoutReason dont_optimize_reason() {
No need to add inline, the compiler should be smart enough to figure that out.

https://codereview.chromium.org/23817003/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/1/src/objects.h#newcode1335
src/objects.h:1335: V(kYield, "Yeld")                                           
               \
No need in \ after the last line

https://codereview.chromium.org/23817003/diff/1/src/objects.h#newcode6601
src/objects.h:6601: inline void set_bailout_reason(BailoutReason reason) {
excessive 'inline'

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
File test/cctest/test-profile-generator.cc (right):

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
test/cctest/test-profile-generator.cc:802: "function kTryFinally() {\n"
Why this "k" prefix?

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
test/cctest/test-profile-generator.cc:821: //         startProfiling
There should always be that tick as we generate it from startProfiling or am I
missing something?

[loislo, 2013-09-02 16:03:44]

comments addressed

https://codereview.chromium.org/23817003/diff/1/include/v8-profiler.h
File include/v8-profiler.h (right):

https://codereview.chromium.org/23817003/diff/1/include/v8-profiler.h#newcode63
include/v8-profiler.h:63: Handle<String> GetBailoutReason() const;
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> Can we start returning const char* instead of JS strings?

Done.

https://codereview.chromium.org/23817003/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/23817003/diff/1/src/ast.h#newcode2316
src/ast.h:2316: inline BailoutReason dont_optimize_reason() {
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> No need to add inline, the compiler should be smart enough to figure that out.

Done.

https://codereview.chromium.org/23817003/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/1/src/objects.h#newcode1335
src/objects.h:1335: V(kYield, "Yeld")                                           
               \
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> No need in \ after the last line

Done.

https://codereview.chromium.org/23817003/diff/1/src/objects.h#newcode6601
src/objects.h:6601: inline void set_bailout_reason(BailoutReason reason) {
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> excessive 'inline'

Done.

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
File test/cctest/test-profile-generator.cc (right):

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
test/cctest/test-profile-generator.cc:802: "function kTryFinally() {\n"
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> Why this "k" prefix?

Done.

https://codereview.chromium.org/23817003/diff/1/test/cctest/test-profile-gene...
test/cctest/test-profile-generator.cc:821: //         startProfiling
On 2013/09/02 14:40:05, Yury Semikhatsky wrote:
> There should always be that tick as we generate it from startProfiling or am I
> missing something?

Done.

[yurys, 2013-09-03 08:20:49]

looks OK to me but I'm not familiar with that code.

[Yang, 2013-09-04 15:18:56]

Two small comments. Looks good so far, but I want to do a second more thorough
pass tomorrow.

https://codereview.chromium.org/23817003/diff/12001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/23817003/diff/12001/src/ast.h#newcode2840
src/ast.h:2840: }
you can put that } on the previous line.

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

https://codereview.chromium.org/23817003/diff/12001/src/profile-generator-inl...
src/profile-generator-inl.h:61: }
Ditto, you could move that } to the previous line.

[Benedikt Meurer, 2013-09-05 07:42:11]

Looks good, but let's wait for Yang's second pass.

https://codereview.chromium.org/23817003/diff/12001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/12001/src/objects.h#newcode1335
src/objects.h:1335: V(kYield, "Yeld")
Shouldn't that read "Yield"?

[loislo, 2013-09-05 07:47:58]

https://codereview.chromium.org/23817003/diff/12001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/23817003/diff/12001/src/ast.h#newcode2840
src/ast.h:2840: }
On 2013/09/04 15:18:56, Yang wrote:
> you can put that } on the previous line.

Done.

https://codereview.chromium.org/23817003/diff/12001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/12001/src/objects.h#newcode1335
src/objects.h:1335: V(kYield, "Yeld")
On 2013/09/05 07:42:11, Benedikt Meurer wrote:
> Shouldn't that read "Yield"?

Done.

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

https://codereview.chromium.org/23817003/diff/12001/src/profile-generator-inl...
src/profile-generator-inl.h:61: }
On 2013/09/04 15:18:56, Yang wrote:
> Ditto, you could move that } to the previous line.

Done.

[Yang, 2013-09-05 12:19:42]

https://codereview.chromium.org/23817003/diff/20001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/20001/src/objects.h#newcode1124
src/objects.h:1124: V(kExportDeclaration, "Export declaration")                 
               \
What do you need this for? Same for the lines below.

[loislo, 2013-09-05 12:44:44]

https://codereview.chromium.org/23817003/diff/20001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/23817003/diff/20001/src/objects.h#newcode1124
src/objects.h:1124: V(kExportDeclaration, "Export declaration")                 
               \
On 2013/09/05 12:19:42, Yang wrote:
> What do you need this for? Same for the lines below.

when v8 parses the js function it may disable optimization for the function
because it found some js statements like try {} catch(e) {}

In that case v8 will never try to optimize it. See the changes in ast.cc.

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/ast.cc&q=fi...

[Yang, 2013-09-05 12:49:18]

On 2013/09/05 12:44:44, loislo wrote:
> https://codereview.chromium.org/23817003/diff/20001/src/objects.h
> File src/objects.h (right):
> 
> https://codereview.chromium.org/23817003/diff/20001/src/objects.h#newcode1124
> src/objects.h:1124: V(kExportDeclaration, "Export declaration")               
 
>                \
> On 2013/09/05 12:19:42, Yang wrote:
> > What do you need this for? Same for the lines below.
> 
> when v8 parses the js function it may disable optimization for the function
> because it found some js statements like try {} catch(e) {}
> 
> In that case v8 will never try to optimize it. See the changes in ast.cc.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/ast.cc&q=fi...

I see.

LGTM.

[loislo, 2013-09-05 13:21:11]

Committed patchset #4 manually as r16555 (presubmit successful).