Separate markbits from heap object map words into bitmaps.

Separate markbits from heap object map words into bitmaps.

This change considerably degrades marking (50% slowdown) and sweeping (25% slowdown) but this is intended.

We are going to reduce overheads in the CLs that will follow.

Committed: http://code.google.com/p/v8/source/detail?r=6230

[Erik Corry, 2011-01-07 12:13:21]

LGTM

http://codereview.chromium.org/6088012/diff/3001/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/6088012/diff/3001/src/heap.cc#newcode218
src/heap.cc:218: int Heap::GcSafeSizeOfOldObject(HeapObject* object) {
Shouldn't we just get rid of this alltogether?

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

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode111
src/mark-compact.cc:111: // Check that swept all marked objects and
that swept -> that we swept
Also formatting is off.

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode543
src/mark-compact.cc:543: ASSERT(!Marking::IsMarked(obj->address()));
I wonder whether the compiler can work out that the -1 that the ->address()
performs can be omitted because the IsMarked shifts away the low bits.

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode554
src/mark-compact.cc:554: static inline bool VisitUnmarkedObjects(Object** start,
Object** end) {
It would be nice to disassemble this function to see whether the heroic inlining
we are expecting of the C++ compiler is actually taking place

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode691
src/mark-compact.cc:691: static inline Map* SafeMap(Object* obj) {
Get rid of this function?

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode1239
src/mark-compact.cc:1239: // Because the object is marked, we have to recover
the original map
comment out of date

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode1421
src/mark-compact.cc:1421: bool MarkCompactCollector::SafeIsMap(HeapObject*
object) {
Remove or rename?

http://codereview.chromium.org/6088012/diff/3001/src/mark-compact.cc#newcode1948
src/mark-compact.cc:1948: int
MarkCompactCollector::SizeOfMarkedObject(HeapObject* obj) {
This function is not needed

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h
File src/spaces.h (right):

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode117
src/spaces.h:117: // Bitmap is a sequence of cells each containing fixed number
of bits.
Perhaps add a comment with the word 'endian' since this stuff looks
endian-dependent.  Or do we only ever access in words, never in bytes?

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode168
src/spaces.h:168: INLINE(void Set(uint32_t index, bool value)) {
I think it's nicer to have Set and Clear methods instead of relying on the
inliner to do that for you.

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode182
src/spaces.h:182: const uint32_t start_mask = (-1) << start;
I think you are assuming here that the rhs of the shift operand will be masked
down, but it would be clearer and less fragile if you did the mask explicitly.

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode189
src/spaces.h:189: cells()[end_cell] &= ~end_mask;
I think this reads and writes (but doesn't change) the word after the end if
start+size hits a word boundary.

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode199
src/spaces.h:199: 
Why two blank lines?

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode280
src/spaces.h:280: static const size_t kBitsPerByteLog2 = 3;
This constant already exists in globals.h

http://codereview.chromium.org/6088012/diff/3001/src/spaces.h#newcode313
src/spaces.h:313: // TODO [EVE] when do we need this crap?
Does this lint or pass the FCC?