http://codereview.chromium.org/6745033/diff/19002/src/heap.cc File src/heap.cc (right):
http://codereview.chromium.org/6745033/diff/19002/src/heap.cc#newcode492 src/heap.cc:492: IncrementalMarking::set_should_hurry(false); On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
I think nicer place to clean the flag is in the
IncrementalMarking::Finalize() Done and made it private. http://codereview.chromium.org/6745033/diff/19002/src/heap.h File src/heap.h (right): http://codereview.chromium.org/6745033/diff/19002/src/heap.h#newcode244 src/heap.h:244: // The fullness of the store buffer when we started to scan the current page. On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
I can't grok the comment.
Rewritten. http://codereview.chromium.org/6745033/diff/19002/src/mark-compact.cc File src/mark-compact.cc (right): http://codereview.chromium.org/6745033/diff/19002/src/mark-compact.cc#newcode1729 src/mark-compact.cc:1729: StoreBuffer::EnterDirectlyIntoStoreBuffer(reinterpret_cast<Address>(p)); On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
This visitor is used to visit new space objects.
Their slots should not be entered into store-buffer.
Good catch! http://codereview.chromium.org/6745033/diff/19002/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/6745033/diff/19002/src/objects-inl.h#newcode834 src/objects-inl.h:834: if (Heap::InNewSpace(value)) \ On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
curly brackets around if's body.
Done. http://codereview.chromium.org/6745033/diff/19002/src/spaces.h File src/spaces.h (right): http://codereview.chromium.org/6745033/diff/19002/src/spaces.h#newcode334 src/spaces.h:334: owner_ = reinterpret_cast<Address>(space) + kFailureTag; On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
add assertion that checks that space was properly aligned.
Done. http://codereview.chromium.org/6745033/diff/19002/src/spaces.h#newcode449 src/spaces.h:449: // The identity of the owning space. This is tagged as a heap pointer, but On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
Update comment. Now this is tagged as a failure and failures can
reside in
objects.
Done. http://codereview.chromium.org/6745033/diff/19002/src/spaces.h#newcode453 src/spaces.h:453: // This flag indicates that the page is not being tracked by the store buffer. On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
Why are we using bool instead of built-in flags?
Because my next step will be to read it from generated code and for that the code will be more compact if it is a bool rather than a bit we have to mask. http://codereview.chromium.org/6745033/diff/19002/src/spaces.h#newcode459 src/spaces.h:459: int store_buffer_counter_; On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
You added 2 fields but kHeaderSize was increased only by 1
kPointerSize. There is a static assert that checks that the size is sufficient. Apparently it was too big before. http://codereview.chromium.org/6745033/diff/19002/src/spaces.h#newcode2051 src/spaces.h:2051: // FALL THROUGH! On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
this comment does not look like something we usually write.
lowercaseified. http://codereview.chromium.org/6745033/diff/19002/src/store-buffer.cc File src/store-buffer.cc (right): http://codereview.chromium.org/6745033/diff/19002/src/store-buffer.cc#newcode171 src/store-buffer.cc:171: ExemptPopularPages(97, ((Page::kPageSize / kPointerSize) / 97) / 8); On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
this looks like a repeatable pattern. loop with array of prime numbers?
Done. http://codereview.chromium.org/6745033/diff/19002/src/store-buffer.cc#newcode231 src/store-buffer.cc:231: containing_chunk = MemoryChunk::FromAnyPointerAddress(addr); On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
This code can be pretty slow on certain patterns.
Yes. http://codereview.chromium.org/6745033/diff/19002/src/store-buffer.cc#newcode340 src/store-buffer.cc:340: void StoreBuffer::Verify() { On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
I think we should have a way to verify store-buffer or we might have
some
troubles debugging strange crashes.
I will consider this for another change. http://codereview.chromium.org/6745033/diff/19002/src/store-buffer.cc#newcode397 src/store-buffer.cc:397: if (array->IsFixedArray()) { On 2011/03/28 15:13:19, Vyacheslav Egorov wrote:
maybe you should assert that array is actually a fixed array, cause no
other lo
chunk should be have scan_on_scavenge().
Done. http://codereview.chromium.org/6745033/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
