Tick processor: Print C++ entry points

Tick processor: Print C++ entry points

R=loislo@chromium.org, yangguo@chromium.org, yurys@chromium.org

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

[Jakob Kummerow, 2014-10-07 15:13:21]

jkummerow@chromium.org changed reviewers:
+ yangguo@chromium.org

[Jakob Kummerow, 2014-10-07 15:13:22]

Alexei, Yury: I'd like to make this change for internal profiling purposes
(--prof + linux-tick-processor). For analyzing big applications/benchmarks, it
can be helpful to know which C++ entry points (say, LoadIC_Miss,
Runtime_ArrayConcat, etc) were expensive in aggregate.
It appears that this CL as is would also affect the public sampling API, as of
https://codereview.chromium.org/596533002. Do you have any suggestions how to
modify the mechanism so the publicly exported information is unchanged? I could
thread through a flag so when we're coming from Sampler::SampleStack(), we'll
record the extra frame, and otherwise (CpuProfiler::StartProfiling() or
V8::GetStackSample()), we won't. Does that sound reasonable?
Or would you even prefer to specifically include these extra frames in public
samples? Personally I think it doesn't make much sense, as such internal details
are likely not interesting to JS developers.

Yang, please review.

[bnoordhuis, 2014-10-07 16:26:37]

On 2014/10/07 15:13:22, Jakob wrote:
> Personally I think it doesn't make much sense, as such internal details
> are likely not interesting to JS developers.

It would be relevant to embedders, or at least, to me.  I frequently see runtime
functions pop up in perf.  It would be nice to have them correlate with JS code.

/outside validation

[Yang, 2014-10-08 13:31:22]

On 2014/10/07 16:26:37, bnoordhuis wrote:
> On 2014/10/07 15:13:22, Jakob wrote:
> > Personally I think it doesn't make much sense, as such internal details
> > are likely not interesting to JS developers.
> 
> It would be relevant to embedders, or at least, to me.  I frequently see
runtime
> functions pop up in perf.  It would be nice to have them correlate with JS
code.
> 
> /outside validation

lgtm.

[loislo, 2014-10-14 15:49:14]

loislo@chromium.org changed reviewers:
+ loislo@chromium.org

[loislo, 2014-10-14 15:49:15]

lgtm with nits

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js
File tools/codemap.js (right):

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js#newcode263
tools/codemap.js:263: CodeMap.CodeEntry = function(size, opt_name, opt_type) {
please annotate

[yurys, 2014-10-15 05:34:49]

On 2014/10/07 15:13:22, Jakob wrote:
> Alexei, Yury: I'd like to make this change for internal profiling purposes
> (--prof + linux-tick-processor). For analyzing big applications/benchmarks, it
> can be helpful to know which C++ entry points (say, LoadIC_Miss,
> Runtime_ArrayConcat, etc) were expensive in aggregate.
> It appears that this CL as is would also affect the public sampling API, as of
> https://codereview.chromium.org/596533002. Do you have any suggestions how to
> modify the mechanism so the publicly exported information is unchanged? I
could
> thread through a flag so when we're coming from Sampler::SampleStack(), we'll
> record the extra frame, and otherwise (CpuProfiler::StartProfiling() or
> V8::GetStackSample()), we won't. Does that sound reasonable?
> Or would you even prefer to specifically include these extra frames in public
> samples? Personally I think it doesn't make much sense, as such internal
details
> are likely not interesting to JS developers.
> 
Yes, that sounds absolutely reasonable to me, let's only record the extra frame
when we are coming from TickSample::Init and skip it otherwise.

I don't think exposing C++ entry points would make much sense without extending
public API that would allow the embedder to map C++ entry point -> function
name. The only way for the embedder to retrieve function name would be to use
debug symbols I guess. I agree that most of the C++ entry points don't make much
sense for JS developers and if we decide to expose them through the public API
we should probably whitelist those that may be interesting for JS developers.

[yurys, 2014-10-15 05:35:47]

yurys@chromium.org changed reviewers:
+ yurys@chromium.org

[yurys, 2014-10-15 05:35:47]

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js
File tools/codemap.js (right):

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js#newcode260
tools/codemap.js:260: * @param {string} opt_name Code entry name.
opt_type parameter annotation is missing

[Jakob Kummerow, 2014-10-15 15:08:57]

Comments addressed. 

The selective recording of the C entry frame made the previous cctest changes
obsolete, as those tests now behave exactly as they did before.

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js
File tools/codemap.js (right):

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js#newcode260
tools/codemap.js:260: * @param {string} opt_name Code entry name.
On 2014/10/15 05:35:47, yurys wrote:
> opt_type parameter annotation is missing

Done.

https://codereview.chromium.org/638633002/diff/1/tools/codemap.js#newcode263
tools/codemap.js:263: CodeMap.CodeEntry = function(size, opt_name, opt_type) {
On 2014/10/14 15:49:15, loislo wrote:
> please annotate

Done.

[yurys, 2014-10-15 17:10:22]

lgtm.

[Jakob Kummerow, 2014-10-17 15:44:24]

Committed patchset #3 (id:20002) manually as 24700 (presubmit successful).