This is looking good. I will compile it into Chromium to smoke-test it.


https://codereview.chromium.org/11420036/diff/1/src/spaces.cc
File src/spaces.cc (right):

https://codereview.chromium.org/11420036/diff/1/src/spaces.cc#newcode2397
src/spaces.cc:2397: int i = 0;
Move the declaration of "i" into to for statement.

https://codereview.chromium.org/11420036/diff/1/src/spaces.cc#newcode2399
src/spaces.cc:2399: for (i = 0; i < kMaxSweepingTries &&
first_unswept_page_->is_valid(); i++) {
We shouldn't check against first_unswept_page_ directly because with
parallel sweeping these accesses need to be synchronized. Better just
use the boolean value that AdvanceSweeper returns to determine whether
to continue or not. Like this ...

const int kMaxSweepingTries = 5;
bool sweeping_complete = false;

for (int i = 0; i < kMaxSweepingTries && !sweeping_complete; i++) {
  sweeping_complete = AdvanceSweeper(size_in_bytes);
  [...]
}

https://codereview.chromium.org/11420036/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to