The names chosen for the various variables confuse me. Apart from that it LGTM.
http://codereview.chromium.org/300036/diff/3001/3005 File src/flag-definitions.h (right): http://codereview.chromium.org/300036/diff/3001/3005#newcode168 Line 168: "max size of (each semispace in) the new generation") The (each semispace in) text looks outdated to me. http://codereview.chromium.org/300036/diff/3001/3007 File src/heap.h (right): http://codereview.chromium.org/300036/diff/3001/3007#newcode254 Line 254: return 2 * young_generation_size_ + old_generation_size_; I think it makes more sense to return the max physical capacity of the heap than the max virtual memory address extent of the heap. So the multiplication by two and the extra comment should be removed. http://codereview.chromium.org/300036/diff/3001/3007#newcode891 Line 891: static int snapshot_semispace_size_; I would prefer these two variables and their associated getters to include the word 'reserved'. Also, since snapshot_semispace_size_ is used even when we are not using snapshots I think it should be called actual_reserved_semispace_size_ and the semispace_size_ variable can then be called requested_reserved_semispace_size_. http://codereview.chromium.org/300036/diff/3001/3007#newcode894 Line 894: static int young_generation_size_; I would prefer this variable and its associated getter to include the syllable 'max'. http://codereview.chromium.org/300036/diff/3001/3006 File src/spaces.cc (right): http://codereview.chromium.org/300036/diff/3001/3006#newcode1001 Line 1001: ASSERT(size == 2 * Heap::SnapshotSemiSpaceSize()); Does this assert apply even if we are not running with snapshots? http://codereview.chromium.org/300036 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
