On 2014/05/08 12:42:40, Yang wrote:
On 2014/05/08 12:40:48, rmcilroy wrote:
> On 2014/05/08 11:13:33, Yang wrote:
> > On 2014/05/08 10:47:47, Yang wrote:
> > > On 2014/05/08 10:47:11, ulan wrote:
> > > > On 2014/05/08 10:36:53, Yang wrote:
> > > > > On 2014/05/08 10:17:09, ulan wrote:
> > > > > > Here is what happens in debug-multiple-breakpoints.js:
> > > > > >
> > > > > > 1. A hot function is marked for optimization by replacing its
code
> with
> > > > > > CompileOptimized or CompileOptimizedConcurrent builtin.
> > > > > > 2. Debug::PrepareForBreakPoints deoptimizes optimized
functions
but
> > > doesn't
> > > > > > restore code of functions that were marked for optimization.
> > > > > > 3. ASSERT_EQ(Code::FUNCTION, function->code()->kind())
triggers in
> > > > > > Debug::MaybeRecompileFunctionForDebugging
> > > > > >
> > > > > > Yang, is this a bug in Debug::PrepareForBreakPoints that
functions
> > marked
> > > > for
> > > > > > optimization do not get their code restored?
> > > > >
> > > > > No. This seems to be a result of the refactoring in
> > > > >
https://codereview.chromium.org/260423002/diff/40001/src/debug.cc
> > > > >
> > > > > The assertion that is triggering has been newly introduced. I
think
we
> > > should
> > > > > remove it. What should happen there is that if the function is
on
the
> > stack
> > > at
> > > > > the point of Debug::PrepareForBreakPoints, it should be
recompiled
for
> > > > debugging
> > > > > regardless of what it currently is (including being marked by
builtin).
> > The
> > > > only
> > > > > exception to that rule is that if it already has been
recompiled for
> > > > debugging.
> > > > >
> > > > > I will make this easier to understand in the future, but for
now,
just
> > > revert
> > > > > that part of the refactoring there.
> > > >
> > > > Thanks, Yang. So we can just remove the assertions
> > > > 2006 ASSERT_EQ(Code::FUNCTION, function->code()->kind());
> > > > 2007 ASSERT_EQ(function->code(), function->shared()->code());
> > > > in Debug::MaybeRecompileFunctionForDebugging? Because the function
code
> will
> > > be
> > > > replaced anyway.
> > >
> > > Yes.
> >
> > Wait. Please check for (Code::FUNCTION == function->code()->kind())
before
> > checking whether it already has_debug_break_slots(). Otherwise the
latter
may
> > assert. Something like
> > if (Code::FUNCTION == function->code()->kind() &&
> > function->code()->has_debug_break_slots()) return;
>
> Thanks Yang! I guess you mean:
> if (Code::FUNCTION != function->code()->kind() ||
> function->code()->has_debug_break_slots()) return;
> ? I'll give that a go and send out a separate CL if this fixes things.
No! I don't mean that. In MaybeRecompileFunctionForDebugging, what we
want to
do
is to make sure the function code afterwards is unoptimized and contains
debug
break slots. So only if that's already true, we return prematurely. If the
function code kind is not FUNCTION, it could be a builtin, or optimized
code.
In
either of those cases, we want to recompile.
We could actually rename it to EnsureFunctionHasDebugBreakSlots.
https://codereview.chromium.org/271543004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
