Unlink evacuation candidates from list of pages before starting sweeper threads.

Unlink evacuation candidates from list of pages before starting sweeper threads.
Removed FinalizeSweeping().

BUG=

Committed: https://code.google.com/p/v8/source/detail?r=13886

[Michael Starzinger, 2013-03-08 12:52:06]

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:563: heap()->FreeQueuedChunks();
There shouldn't be any queued chunks at this point, I think this call is
obsolete.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3323: void
MarkCompactCollector::UnlinkEvacuationCandidates() {
Can we move that to above the ReleaseEvacuationCandidates because that's the
natural order in which it gets called.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3908: UnlinkEvacuationCandidates();
Add a short comment about why this needs to be done before the sweeper threads
start.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3935: ReleaseEvacuationCandidates();
The right place to call FreeQueuedChunks would be after we release evacuation
candidates, it might even make sense to move it into
ReleaseEvacuationCandidates().

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.h#newcode717
src/mark-compact.h:717: void UnlinkEvacuationCandidates();
Can we move that up one line because that's the natural order in which it gets
called.

https://codereview.chromium.org/12499004/diff/3001/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/12499004/diff/3001/src/spaces.cc#newcode550
src/spaces.cc:550: if (next_chunk_ != NULL) {
This would indicate that you try to unlink a Page twice, this should not happen.
Can we revert this part of the change?

[Hannes Payer (out of office), 2013-03-08 14:02:30]

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:563: heap()->FreeQueuedChunks();
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> There shouldn't be any queued chunks at this point, I think this call is
> obsolete.

Done.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3323: void
MarkCompactCollector::UnlinkEvacuationCandidates() {
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> Can we move that to above the ReleaseEvacuationCandidates because that's the
> natural order in which it gets called.

Done.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3908: UnlinkEvacuationCandidates();
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> Add a short comment about why this needs to be done before the sweeper threads
> start.

Done.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:3935: ReleaseEvacuationCandidates();
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> The right place to call FreeQueuedChunks would be after we release evacuation
> candidates, it might even make sense to move it into
> ReleaseEvacuationCandidates().

Done.

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/12499004/diff/3001/src/mark-compact.h#newcode717
src/mark-compact.h:717: void UnlinkEvacuationCandidates();
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> Can we move that up one line because that's the natural order in which it gets
> called.

Done.

https://codereview.chromium.org/12499004/diff/3001/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/12499004/diff/3001/src/spaces.cc#newcode550
src/spaces.cc:550: if (next_chunk_ != NULL) {
On 2013/03/08 12:52:06, Michael Starzinger wrote:
> This would indicate that you try to unlink a Page twice, this should not
happen.
> Can we revert this part of the change?

Done.

[Michael Starzinger, 2013-03-08 14:20:52]

LGTM with nits. Much better than the previous approach.

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.cc#newcod...
src/mark-compact.cc:3909: // pages.
Add a "... to avoid race condition." here.

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.h#newcode693
src/mark-compact.h:693: void FinalizeSweeping();
The implementation of this method is gone, let's also remove the declaration.

[Hannes Payer (out of office), 2013-03-08 14:34:44]

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.cc#newcod...
src/mark-compact.cc:3909: // pages.
On 2013/03/08 14:20:52, Michael Starzinger wrote:
> Add a "... to avoid race condition." here.

Done.

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/12499004/diff/8002/src/mark-compact.h#newcode693
src/mark-compact.h:693: void FinalizeSweeping();
On 2013/03/08 14:20:52, Michael Starzinger wrote:
> The implementation of this method is gone, let's also remove the declaration.

Done.

[Hannes Payer (out of office), 2013-03-08 14:41:28]

Committed patchset #5 manually as r13886 (presubmit successful).