On 2014/08/22 17:20:58, Denis Pravdin wrote:
On 2014/08/19 14:23:35, alph wrote:
>
https://codereview.chromium.org/424973004/diff/120001/test/cctest/test-cpu-profiler.cc
> File test/cctest/test-cpu-profiler.cc (right):
>
>
https://codereview.chromium.org/424973004/diff/120001/test/cctest/test-cpu-profiler.cc#newcode1134
> test/cctest/test-cpu-profiler.cc:1134: CHECK_NE(NULL, line_info);
> On 2014/08/15 20:57:55, Denis Pravdin wrote:
> > On 2014/08/15 12:10:47, alph wrote:
> > > I've got the test failing at this line for ia32 target because
there's
no
> > > POSITION reloc infos for the code.
> >
> > Well, I found out what causes this test failure but I don't
understand a
> reason
> > of this behavior and I am not sure what the best way to fix it is:
> >
> > In patch set 3, I iterate the heap to get a function object
> > (GetFunctionInfoFromHeap). The test passes all the time.
> >
> > In patch set 4, I replace it with the lines below as you suggested
(see a
code
> > snippet below) and test failed.
> >
> > i::Handle<i::JSFunction> func = v8::Utils::OpenHandle(
> > *v8::Local<v8::Function>::Cast(
> > (*env)->Global()->Get(v8_str(func_name))));
> >
> > Looking into it, I decided to check what the difference between these
two
> ways:
> > 1. I added heap iteration back in addition to v8::Utils::OpenHandle
> > 2. Then checked that reloc info for both objects of Code type include
> positions.
> >
> > The test looks like:
> >
> > CompileRun(script.start());
> >
> > i::SharedFunctionInfo* shared = GetFunctionInfoFromHeap(isolate,
func_name);
> > int positions_ = 0;
> > for (i::RelocIterator it(shared->code()); !it.done(); it.next()) {
> > i::RelocInfo::Mode mode = it.rinfo()->rmode();
> > if (i::RelocInfo::IsPosition(mode)) positions_++;
> > }
> > CHECK_GT(positions_, 0); // THIS CHECK PASSED
> >
> > i::Handle<i::JSFunction> func = v8::Utils::OpenHandle(
> > *v8::Local<v8::Function>::Cast(
> > (*env)->Global()->Get(v8_str(func_name))));
> > CHECK_NE(NULL, func->shared());
> > CHECK_NE(NULL, func->code());
> > int positions = 0;
> > for (i::RelocIterator it(func->code()); !it.done(); it.next()) {
> > i::RelocInfo::Mode mode = it.rinfo()->rmode();
> > if (i::RelocInfo::IsPosition(mode)) positions++;
> > }
> > CHECK_GT(positions, 0); // THIS CHECK DOES NOT PASS.
> >
> > Do you have idea why for the first case I have POSITION reloc info,
and
never
> > have it for the second case?
>
> Could you please check the 'shared' and 'code' objects you get using
these
two
> approaches are the same.
>
> I think there are several versions on the code for the function, e.g.
optimized
> and non-optimized. One of them is missing relocations.
>
Yes, you're right.
I debugged the test and found out:
1. There are two ways to get 'code' using JSFunction object (jsf is type
of
JSFunction):
(a) jsf->code()
(b) jsf->shared()->code()
2. When optimized code is compiled that both JSFunction::ReplaceCode(Code*
code)
and SharedFunctionInfo::ReplaceCode(Code* value) function are called. As
the
result, the 'code' objects returned by (a) and (b) contains relocations.
3. Then, optimized version of code is generated and only
SFunction::ReplaceCode(Code* code) is called.
Note that I use func->code() instead of func->shared()->code():
for (i::RelocIterator it(func->code()); !it.done(); it.next()) {
i::RelocInfo::Mode mode = it.rinfo()->rmode();
if (i::RelocInfo::IsPosition(mode)) positions++;
}
That's why my previous approach (GetFunctionInfoFromHeap) always works
but new
one (OpenHandle) causes a test failure.
Possible solutions:
1) For testing purpose, always use unoptimized version of code (e.g.
func->shared()->code())
+ allows to make the test more deterministic
- optimized code is not tested
2) Set FLAG_hydrogen_track_positions flag as true by default before
CompileRun
for this test.
I checked on Windows and see that number of positions is increased
from 9
to
19 for 'func' function from the test.
I think that the flag should be turned on always to achieve better
mapping
to
source lines for generated optimized code. We can add a runtime switch to
turn
it on before profiling starts. It's a subject for a separate patch.
Can you suggest other ideas? If not that which of the above is the best?
> >
> > It seems it's Linux specific issue. At least I don't see it on
Windows.
Any comments?
https://codereview.chromium.org/424973004/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
