http://codereview.chromium.org/9634005/diff/1/src/spaces.cc File src/spaces.cc (left):
http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#oldcode2625 src/spaces.cc:2625: accidental edit? please revert. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2523 src/spaces.cc:2523: static bool match(void* key1, void* key2) { Please give this function a more meaningful name, e.g. ComparePointers/SamePointers/etc http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2526 src/spaces.cc:2526: one additional empty line http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2588 src/spaces.cc:2588: // map following entries to this page object, in which keys are comments should start with a capital letter and end with a dot. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2588 src/spaces.cc:2588: // map following entries to this page object, in which keys are I would rephrase this comment, for example: // Register all MemoryChunk::kAlignment-aligned chunks covered by this large page in the chunk map. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2595 src/spaces.cc:2595: key += MemoryChunk::kAlignment, hash++) { I think you can just unify hash and key (make key == hash) because key is aligned and it's 20 less significant bits are always zeros. This will make this loop cleaner. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2596 src/spaces.cc:2596: (map_.Lookup(reinterpret_cast<void*>(key), hash, true))->value = page; I would prefer this is done in two lines instead of a single line. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2624 src/spaces.cc:2624: if ( page_address <= a && a < page_address + page->size() ) { accidental edit? please revert. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2642 src/spaces.cc:2642: return page; This code is duplicated in two places. Please move everything to a single helper function. http://codereview.chromium.org/9634005/diff/1/src/spaces.cc#newcode2679 src/spaces.cc:2679: // remove entries belonging to this page comments should start with a capital letter and end with a dot. http://codereview.chromium.org/9634005/diff/1/src/spaces.h File src/spaces.h (right): http://codereview.chromium.org/9634005/diff/1/src/spaces.h#newcode2540 src/spaces.h:2540: HashMap map_; // speed up containing pointer check Consider more meaningful name and comment. For example: // Map MemoryChunk::kAlignment-aligned chunks to large pages covering them. chunk_map_; http://codereview.chromium.org/9634005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
