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.
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.
