CpuProfiler: move InlinedFunctionInfo class from HGraphBuilder to CompilationInfo.

CpuProfiler: move InlinedFunctionInfo class from HGraphBuilder to CompilationInfo.

A function could be deoptimized due to a deopt in the inlined code.
The inlined function might be defined in another script. So we need to
track the information about the inlined functions (scriptId and offset).
We already have the tracking code which is behind FLAG_hydrogen_track_position.
So as the first step we need to make the info accessible by CPU profiler.
In the follow-up patches I'll add the code which will enable position
tracking and push the info into CodeEntry entries.

BUG=452067
LOG=n

Committed: https://crrev.com/e60ec273cca2c542bb98ef7f97dd4af7d4b84e5a
Cr-Commit-Position: refs/heads/master@{#26680}

[loislo, 2015-02-16 13:20:01]

loislo@chromium.org changed reviewers:
+ alph@chromium.org, svenpanne@chromium.org, vegorov@chromium.org,
yurys@chromium.org

[loislo, 2015-02-16 13:20:02]

PTAL

[Vyacheslav Egorov (Google), 2015-02-16 13:38:49]

vegorov@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2015-02-16 13:38:50]

Some comments from me.

I would like to also note that this tracking was designed with two requirements
in mind:

- to work using whatever source is dumped, without access to full *live* source;

- to be path sensitive, e.g. if you have function 

function foo() {
  bar();
  bar();
}

and you inline bar() twice and the then you deopt from inside the second
invocation to bar() then you would see precisely that.

I am not sure if you need the same granularity for Dev Tools - I just don't know
how UI is going to look like for these deopts.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode204
src/compiler.cc:204: if (inlined_function_infos_) delete
inlined_function_infos_;
delete NULL works just fine - so no need for if.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode338
src/compiler.cc:338: if (!shared->script()->IsUndefined()) {
Given that you want to reuse this function for profiler - it makes sense to
guard this branch proactively with FLAG_... to avoid spitting output when flag
is disabled.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode341
src/compiler.cc:341: CodeTracer::Scope
tracing_scopex(isolate()->GetCodeTracer());
can it be called just tracing_scope here?

https://codereview.chromium.org/914413007/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/914413007/diff/1/src/hydrogen.cc#newcode3497
src/hydrogen.cc:3497: int inline_id =
info()->inlining_id_to_function_id()->length();
Should not all this code be also moved into GetInlinedFunctionId? (including the
tracing down there).

[loislo, 2015-02-16 14:48:32]

I saw IRHydra2 page. The idea is the same but there is no UI yet. The only
difference is that DevTools has full sources, probably with source maps so I
need to push absolute offset to the cpu profile.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode204
src/compiler.cc:204: if (inlined_function_infos_) delete
inlined_function_infos_;
On 2015/02/16 13:38:50, Vyacheslav Egorov (Google) wrote:
> delete NULL works just fine - so no need for if.

Done.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode338
src/compiler.cc:338: if (!shared->script()->IsUndefined()) {
On 2015/02/16 13:38:50, Vyacheslav Egorov (Google) wrote:
> Given that you want to reuse this function for profiler - it makes sense to
> guard this branch proactively with FLAG_... to avoid spitting output when flag
> is disabled.

Yep. I supposed to do that in the next patch.

https://codereview.chromium.org/914413007/diff/1/src/compiler.cc#newcode341
src/compiler.cc:341: CodeTracer::Scope
tracing_scopex(isolate()->GetCodeTracer());
On 2015/02/16 13:38:50, Vyacheslav Egorov (Google) wrote:
> can it be called just tracing_scope here?

Done.

https://codereview.chromium.org/914413007/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/914413007/diff/1/src/hydrogen.cc#newcode3497
src/hydrogen.cc:3497: int inline_id =
info()->inlining_id_to_function_id()->length();
On 2015/02/16 13:38:50, Vyacheslav Egorov (Google) wrote:
> Should not all this code be also moved into GetInlinedFunctionId? (including
the
> tracing down there).

Done.

[Vyacheslav Egorov (Google), 2015-02-16 15:12:03]

LGTM (though it's kinda useless, I am not in OWNERs)

it would be good if you could run the test suite in the debug mode with
--hydrogen-track-position and see what/if something fails.

https://codereview.chromium.org/914413007/diff/20001/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/914413007/diff/20001/src/compiler.h#newcode33
src/compiler.h:33: class InlinedFunctionInfo {
Maybe we should move HSourcePosition and rename into something like
SourcePosition, OptSourcePosition, SourceLocation, etc; to here as well so that
we could use it instead of raw intptr_t value?

[Sven Panne, 2015-02-17 07:31:06]

LGTM, but please take care of Slava's suggestion in a later CL.

https://codereview.chromium.org/914413007/diff/20001/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/914413007/diff/20001/src/compiler.h#newcode33
src/compiler.h:33: class InlinedFunctionInfo {
On 2015/02/16 15:12:02, Vyacheslav Egorov (Google) wrote:
> Maybe we should move HSourcePosition and rename into something like
> SourcePosition, OptSourcePosition, SourceLocation, etc; to here as well so
that
> we could use it instead of raw intptr_t value?
> 

This sounds sensible, I don't really like all those naked integral values with
tons of different interpretations. While you're there, please fix the undefined
behavior in HSourcePosition (found by -fsanitize=shift): BitFields should
*never* wrap a signed type.

[loislo, 2015-02-17 08:02:46]

New patchsets have been uploaded after l-g-t-m from
vegorov@google.com,svenpanne@chromium.org

[loislo, 2015-02-17 08:03:52]

The CQ bit was checked by loislo@chromium.org

[commit-bot: I haz the power, 2015-02-17 08:04:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/914413007/40001

[loislo, 2015-02-17 08:07:59]

The CQ bit was unchecked by loislo@chromium.org

[loislo, 2015-02-17 09:43:51]

The CQ bit was checked by loislo@chromium.org

[commit-bot: I haz the power, 2015-02-17 09:44:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/914413007/40001

[commit-bot: I haz the power, 2015-02-17 09:44:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-17 09:45:03]

Patchset 3 (id:??) landed as
https://crrev.com/e60ec273cca2c542bb98ef7f97dd4af7d4b84e5a
Cr-Commit-Position: refs/heads/master@{#26680}