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

https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode84
src/compiler.cc:84: explicit
CompilationInfoWithZone(Handle<SharedFunctionInfo> shared)
On 2015/07/20 11:29:40, Michael Starzinger wrote:
See comment below.

Acknowledged.

https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1001
src/compiler.cc:1001: CompilationInfoWithZone info(shared);
On 2015/07/20 11:29:39, Michael Starzinger wrote:
There is a constant back-and-forth refactoring around
CompilationInfoWithZone.
Can we make the following lines explicit here to avoid adding a new
constructor:

     Zone zone;
     ParseInfo parse_info(&zone, shared);
     CompilationInfo info(&parse_info);

Done.

https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1394
src/compiler.cc:1394: allow_lazy &=
!LiveEditFunctionTracker::IsActive(isolate);
On 2015/07/20 11:29:39, Michael Starzinger wrote:
nit: Can we combine this into one big conjunction instead?

Done.

https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1464
src/compiler.cc:1464: DCHECK(info.is_debug() ? !existing->HasDebugCode()
On 2015/07/20 11:29:39, Michael Starzinger wrote:
nit: Not sure exactly what this assertion is guarding against, I would
be fine
with dropping it entirely.

This is supposed to guard against replacing existing debug code with a
new copy. In that case we would lose break points, as the new code is
not patched.

https://codereview.chromium.org/1233073005/diff/1/src/compiler.cc#newcode1560
src/compiler.cc:1560: DCHECK(!shared->HasDebugInfo());
On 2015/07/20 11:29:39, Michael Starzinger wrote:
What happens of a concurrent compile job for a function has been
enqueued and
the debugger sets a breakpoint inside that function while it's on the
queue.
Does the debugger flush the queue somehow? Or is there another
mechanism in
place to prevent this?

Done. We flush the queue when preparing the function for break points.

https://codereview.chromium.org/1233073005/diff/1/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/1233073005/diff/1/src/compiler.h#newcode630
src/compiler.h:630: MUST_USE_RESULT static MaybeHandle<Code>
CompileForDebugging(
On 2015/07/20 11:29:40, Michael Starzinger wrote:
What do you think about sticking with GetDebugCode as the name for
both of these
function, to keep it in sync with the rest of the API methods?

Done.

https://codereview.chromium.org/1233073005/diff/1/src/compiler/js-inlining.cc
File src/compiler/js-inlining.cc (right):

https://codereview.chromium.org/1233073005/diff/1/src/compiler/js-inlining.cc#newcode254
src/compiler/js-inlining.cc:254: return NoChange();
On 2015/07/20 11:29:40, Michael Starzinger wrote:
Please add a tracing line ...

TRACE("Not inlining %s into %s because callee contains breakpoints\n",
...)

Done.

https://codereview.chromium.org/1233073005/diff/1/src/debug.cc
File src/debug.cc (left):

https://codereview.chromium.org/1233073005/diff/1/src/debug.cc#oldcode1626
src/debug.cc:1626: heap->CollectAllGarbage(Heap::kMakeHeapIterableMask,
On 2015/07/20 11:46:09, Michael Starzinger wrote:
As discussed offline: We might need to still abort incremental marking
explicitly, to disable code flushing.

Done.

https://codereview.chromium.org/1233073005/diff/1/src/heap/objects-visiting-inl.h
File src/heap/objects-visiting-inl.h (right):

https://codereview.chromium.org/1233073005/diff/1/src/heap/objects-visiting-inl.h#newcode663
src/heap/objects-visiting-inl.h:663: if (shared_info->HasDebugInfo()) {
On 2015/07/20 11:33:41, Michael Starzinger wrote:
Ouch, this is scary! Is this a hard requirement? If it is then we need
to make
double-sure that flushing decisions are revisited whenever this
predicate
changes. Otherwise the SharedFunctionInfo might be enqueued as a
flushing
candidate already when the debugger adds breakpoints.

Done.

https://codereview.chromium.org/1233073005/

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

Reply via email to