http://codereview.chromium.org/8050010/diff/2001/src/compiler.cc
File src/compiler.cc (right):
http://codereview.chromium.org/8050010/diff/2001/src/compiler.cc#newcode314
src/compiler.cc:314: if (info->CompilingForDebugging()) {
On 2011/09/29 10:47:35, Kevin Millikin wrote:
You could piggyback on the existing test so the control flow through
here
doesn't get too complicated conditions don't get too complicated:
return V8::UseCrankshaft() && !info->CompilingForDebugging()
? MakeCrankshaftCode(info)
: FullCodeGenerator::MakeCode(info);
or
return info->CompilingForDebugging() || !V8::UseCrankshaft()
? FullCodeGenerator::MakeCode(info)
: MakeCrankshaftCode(info);
Done.
http://codereview.chromium.org/8050010/diff/2001/src/compiler.h
File src/compiler.h (right):
http://codereview.chromium.org/8050010/diff/2001/src/compiler.h#newcode250
src/compiler.h:250: bool compiling_for_debugging_;
On 2011/09/29 10:47:35, Kevin Millikin wrote:
There's a bit field "flags_" in this class. I think this bit (as well
as
supports_deoptimization_) should be packed in there.
Conveniently, the flags all default to false.
Good point - changed.
http://codereview.chromium.org/8050010/diff/2001/src/debug.cc
File src/debug.cc (right):
http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1734
src/debug.cc:1734: static int HandleObjectPointerCompare(const
Handle<T>* a, const Handle<T>* b) {
On 2011/09/29 10:47:35, Kevin Millikin wrote:
Hmm. There are a couple of other Compare functions in utils.h. I
don't think
this can go there, because of include dependencies, but maybe it
should go in
handles.h with a comment in utils.h that it's in handles.h, and a
comment in
handles.h that it's like the ones in utils.h but moved because of
include
dependencies.
I'm worried someone else will just define the same function, because
debug.cc is
a strange place to look for it. Or maybe that's not a big deal. What
do you
think?
Good point. I moved it to utils.h together with a forward declaration of
Handle.
http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1782
src/debug.cc:1782: Handle<Code> lazy_compile(
On 2011/09/29 10:47:35, Kevin Millikin wrote:
Handle<Code> lazy_compile = isolate_->builtins()->LazyCompile();
Done, actually
Handle<Code> lazy_compile =
Handle<Code>(isolate_->builtins()->LazyCompile());
http://codereview.chromium.org/8050010/diff/2001/src/debug.cc#newcode1813
src/debug.cc:1813: while (((obj = iterator.next()) != NULL)) {
On 2011/09/29 10:47:35, Kevin Millikin wrote:
Maybe it doesn't matter, but should you avoid scanning map, code, and
data
spaces?
Currently there is no space filtering in the heap iterator. I will
prepare one as a separate change.
http://codereview.chromium.org/8050010/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
