Thanks a lot for review, Søren, committing.
http://codereview.chromium.org/509035/diff/5006/5008 File src/mark-compact.cc (right): http://codereview.chromium.org/509035/diff/5006/5008#newcode1412 src/mark-compact.cc:1412: ASSERT(Heap::map_space()->Contains(new_map)); On 2010/01/13 08:55:30, Søren Gjesse wrote:
On 2010/01/12 16:53:36, antonm wrote: > On 2010/01/12 08:07:14, Søren Gjesse wrote: > > Does this ASSERT check against the resulting size of map space
after
> compaction? > > Very good question. I don't think so as the space shouldn't have
been shrunk
> yet. > > I've added the check, but it feels somewhat ugly to me (esp. this recalculation > of # of live maps). Maybe we'd better add this check (if it's not
present
right > now) into post GC verification code and remove it from here not to
confuse
> readers? >
You are right it it both ugly and slow. Your suggestion to add some
map space
verification sounds much better,
Ok. I double checked and it looks like all the cctest are ran with verify_heap flag on, so we should get it for free. Just removing. http://codereview.chromium.org/509035/diff/5006/5008#newcode1483 src/mark-compact.cc:1483: MapCompact map_compact(from_space); On 2010/01/13 08:55:30, Søren Gjesse wrote:
On 2010/01/12 16:53:36, antonm wrote: > On 2010/01/12 08:07:14, Søren Gjesse wrote: > > How about passing in Heap::map_space() explicitly as the to space
here?
> > Sorry, don't quite understand you, could you elaborate? > > In case it's due to bad naming, just dropped the var :)
My suggenstion was to add another parameter indicating the from space,
but you
can ignore that. Thinking about it again how about dropping the
current
parameter? It's up to you.
Good idea, done. I used to depend on it in the body before, but not now. Note: couldn't drop it completely as it depends on live map count. http://codereview.chromium.org/509035
-- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
