Improved, safer handling of the symbol table. The symbols themselves...

Improved, safer handling of the symbol table.  The symbols themselves
are not treated as roots, but all their subparts are.

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

[iposva, 2009-05-05 05:05:19]

LGTM wih comments.

-Ivan

http://codereview.chromium.org/108002/diff/1/3
File src/mark-compact.cc (right):

http://codereview.chromium.org/108002/diff/1/3#newcode570
Line 570: if (!(*p)->IsHeapObject()) continue;
Can there be non-heap entries in the symbol table? I guess the table is
initialized to zeroes, but it would be nice to comment it here, so that I and
others don't have to figure that out everytime we read this code.

http://codereview.chromium.org/108002/diff/1/2
File src/mark-compact.h (right):

http://codereview.chromium.org/108002/diff/1/2#newcode178
Line 178: // Mark the symbol table specially.  References to symbols are weak.
Please clarify: References to symbols from the symbol table are weak.

[Kasper Lund, 2009-05-05 05:22:33]

LGTM, but

http://codereview.chromium.org/108002/diff/1/3
File src/mark-compact.cc (right):

http://codereview.chromium.org/108002/diff/1/3#newcode572
Line 572: HeapObject* object = HeapObject::cast(*p);
The newlines here look weird. How about moving the HeapObject* object definition
below the comment?

http://codereview.chromium.org/108002/diff/1/3#newcode580
Line 580: object->IterateBody(map->instance_type(),
I'd compute the size before the IterateBody call, stuff it in a local variable,
and use it to get nicely formatted code where the IterateBody call fits on one
line. But that's just me.

http://codereview.chromium.org/108002/diff/1/2
File src/mark-compact.h (right):

http://codereview.chromium.org/108002/diff/1/2#newcode159
Line 159: UpdateLiveObjectCount(obj, 1);
Do you need the scale argument to UpdateLiveObjectCount anymore?

[Kevin Millikin (Chromium), 2009-05-05 05:34:07]

http://codereview.chromium.org/108002/diff/1/3
File src/mark-compact.cc (right):

http://codereview.chromium.org/108002/diff/1/3#newcode572
Line 572: HeapObject* object = HeapObject::cast(*p);
On 2009/05/05 05:22:34, Kasper Lund wrote:
> The newlines here look weird. How about moving the HeapObject* object
definition
> below the comment?

I could do that, but the comment goes with the continue.

http://codereview.chromium.org/108002/diff/1/3#newcode580
Line 580: object->IterateBody(map->instance_type(),
On 2009/05/05 05:22:34, Kasper Lund wrote:
> I'd compute the size before the IterateBody call, stuff it in a local
variable,
> and use it to get nicely formatted code where the IterateBody call fits on one
> line. But that's just me.

That's not the way I roll.

http://codereview.chromium.org/108002/diff/1/2
File src/mark-compact.h (right):

http://codereview.chromium.org/108002/diff/1/2#newcode159
Line 159: UpdateLiveObjectCount(obj, 1);
On 2009/05/05 05:22:34, Kasper Lund wrote:
> Do you need the scale argument to UpdateLiveObjectCount anymore?

No, you're right.  I'll revert it when we clean this all up.