Thanks. Committing.
http://codereview.chromium.org/6542047/diff/1/src/SConscript File src/SConscript (right): http://codereview.chromium.org/6542047/diff/1/src/SConscript#newcode86 src/SConscript:86: incremental-marking.cc On 2011/02/22 12:27:19, Erik Corry wrote:
c is before s in the latin alphabet.
Done. http://codereview.chromium.org/6542047/diff/1/src/arm/deoptimizer-arm.cc File src/arm/deoptimizer-arm.cc (right): http://codereview.chromium.org/6542047/diff/1/src/arm/deoptimizer-arm.cc#newcode124 src/arm/deoptimizer-arm.cc:124: void Deoptimizer::PatchStackCheckCodeAt(Code* code, On 2011/02/22 12:27:19, Erik Corry wrote:
Should we add a TODO to make use of this argument?
Done. http://codereview.chromium.org/6542047/diff/1/src/assembler.h File src/assembler.h (right): http://codereview.chromium.org/6542047/diff/1/src/assembler.h#newcode274 src/assembler.h:274: INLINE(void set_target_address(Address target, Code* code)); On 2011/02/22 12:27:19, Erik Corry wrote:
Could you use GetCodeFromTargetAddress instead of adding an argument?
Actually it seems that they are not the same object. Perhaps a better
name than
'code' can be found.
Done. http://codereview.chromium.org/6542047/diff/1/src/compiler-intrinsics.h File src/compiler-intrinsics.h (right): http://codereview.chromium.org/6542047/diff/1/src/compiler-intrinsics.h#newcode1 src/compiler-intrinsics.h:1: // Copyright 2010 the V8 project authors. All rights reserved. On 2011/02/22 12:27:19, Erik Corry wrote:
It seems to me that this should either be unchanged or 2011.
Done. http://codereview.chromium.org/6542047/diff/1/src/debug.cc File src/debug.cc (right): http://codereview.chromium.org/6542047/diff/1/src/debug.cc#newcode376 src/debug.cc:376: Handle<Code> code(Code::GetCodeFromTargetAddress(target)); On 2011/02/22 12:27:19, Erik Corry wrote:
Perhaps rename this variable to something more meaningful, then you
can replace
this->code() with just code() below.
Done. http://codereview.chromium.org/6542047/diff/1/src/deoptimizer.h File src/deoptimizer.h (right): http://codereview.chromium.org/6542047/diff/1/src/deoptimizer.h#newcode142 src/deoptimizer.h:142: static void PatchStackCheckCodeAt(Code* code, On 2011/02/22 12:27:19, Erik Corry wrote:
Call this variable unoptimized_code?
Done. http://codereview.chromium.org/6542047/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/6542047/diff/1/src/heap.cc#newcode476 src/heap.cc:476: PrintF("[IncremenalMarker] SCAVENGE -> MARK-SWEEEP\n"); On 2011/02/22 12:27:19, Erik Corry wrote:
menal -> mental SWEEEP -> SWEEP
Done. http://codereview.chromium.org/6542047/diff/1/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/6542047/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode1961 src/ia32/lithium-codegen-ia32.cc:1961: __ IncrementalMarkingRecordWrite(object, value, scratch); On 2011/02/22 12:27:19, Erik Corry wrote:
This looks rather horrible in terms of some performance issues. We
should
probably experiment with rescanning the cell space at the end of the
incremental
marking.
Yes. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h File src/incremental-marking.h (right): http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode48 src/incremental-marking.h:48: static void Start() { On 2011/02/22 12:27:19, Erik Corry wrote:
I think it would be cleaner to move largish functions that are not
inline/not
called very often into the .cc file. This includes Start() and
Hurry(). Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode89 src/incremental-marking.h:89: state_ = STOPPED; On 2011/02/22 12:27:19, Erik Corry wrote:
Fits on one line.
Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode129 src/incremental-marking.h:129: static const intptr_t kFactor = 8; On 2011/02/22 12:27:19, Erik Corry wrote:
Perhaps call this kAllocationMarkingFactor.
Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode221 src/incremental-marking.h:221: // Grey markbits: 01 On 2011/02/22 12:27:19, Erik Corry wrote:
Probably should be 11 when we handle overflow.
Yes. Will investigate when we start handling overflow. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode255 src/incremental-marking.h:255: return "???"; On 2011/02/22 12:27:19, Erik Corry wrote:
UNREACHABLE()?
Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode303 src/incremental-marking.h:303: WhiteToGrey(object); On 2011/02/22 12:27:19, Erik Corry wrote:
This pushes the object onto the stack. When it is popped the map will
be
marked.
Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode305 src/incremental-marking.h:305: if (IsWhite(map)) WhiteToGrey(map); On 2011/02/22 12:27:19, Erik Corry wrote:
Here we also mark the map. Aren't we doing it twice?
Done. http://codereview.chromium.org/6542047/diff/1/src/incremental-marking.h#newcode321 src/incremental-marking.h:321: // We are sweeping code and map spaces precisely so clearing is not required. On 2011/02/22 12:27:19, Erik Corry wrote:
lint?
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc File src/mark-compact.cc (right): http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode105 src/mark-compact.cc:105: On 2011/02/22 12:27:19, Erik Corry wrote:
2 blank lines here and several places.
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode155 src/mark-compact.cc:155: // if (IsCompacting()) tracer_->set_is_compacting(); On 2011/02/22 12:27:19, Erik Corry wrote:
Commented code.
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode232 src/mark-compact.cc:232: // on top of it to see whether they are equal to old_start. On 2011/02/22 12:27:19, Erik Corry wrote:
Or perhaps we should compact the marking stack by removing pointers to
filler
objects in this case.
We will think about it. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode892 src/mark-compact.cc:892: UNREACHABLE(); On 2011/02/22 12:27:19, Erik Corry wrote:
TODO?
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode893 src/mark-compact.cc:893: #if 0 On 2011/02/22 12:27:19, Erik Corry wrote:
Commented code.
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode1175 src/mark-compact.cc:1175: /*if (FLAG_collect_maps && On 2011/02/22 12:27:19, Erik Corry wrote:
Commented code. TODO?
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode1515 src/mark-compact.cc:1515: // FlushCode::ProcessCandidates(); On 2011/02/22 12:27:19, Erik Corry wrote:
Commented code and missing TODO'
Done. http://codereview.chromium.org/6542047/diff/1/src/mark-compact.cc#newcode2006 src/mark-compact.cc:2006: object->Size() == 4 || On 2011/02/22 12:27:19, Erik Corry wrote:
kPointerSize?
Done. http://codereview.chromium.org/6542047/diff/1/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/6542047/diff/1/src/objects-inl.h#newcode84 src/objects-inl.h:84: WRITE_BARRIER(this, offset, value); \ On 2011/02/22 12:27:19, Erik Corry wrote:
Indentation of backslashes is now wrong.
Done. http://codereview.chromium.org/6542047/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
