Estimate a (close) upper bound on the size of black-marked objects on each page.

src/ia32/macro-assembler-ia32.cc

Index: src/ia32/macro-assembler-ia32.cc
diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index 561965717cf8261e171778c76e1480eb4fb092b4..9907ee51a8a970d556be80a27b8e53cbbffcab63 100644
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -2331,35 +2331,12 @@ void MacroAssembler::HasColor(Register object,
 }
 
 
-// Detect some, but not all, common pointer-free objects.  This is used by the
-// incremental write barrier which doesn't care about oddballs (they are always
-// marked black immediately so this code is not hit).
-void MacroAssembler::JumpIfDataObject(
-    Register value,
-    Register scratch,
-    Label* not_data_object,
-    Label::Distance not_data_object_distance) {
-  Label is_data_object;
-  mov(scratch, FieldOperand(value, HeapObject::kMapOffset));
-  cmp(scratch, FACTORY->heap_number_map());
-  j(equal, &is_data_object, Label::kNear);
-  ASSERT(kConsStringTag == 1 && kIsConsStringMask == 1);
-  ASSERT(kNotStringTag == 0x80 && kIsNotStringMask == 0x80);
-  // If it's a string and it's not a cons string then it's an object containing
-  // no GC pointers.
-  test_b(FieldOperand(scratch, Map::kInstanceTypeOffset),
-         kIsConsStringMask | kIsNotStringMask);
-  j(not_zero, not_data_object, not_data_object_distance);
-  bind(&is_data_object);
-}
-
-
 void MacroAssembler::GetMarkBits(Register addr_reg,
                                  Register bitmap_reg,
                                  Register mask_reg) {
-  ASSERT(!AreAliased(addr_reg, bitmap_reg, mask_reg, ecx));
-  mov(bitmap_reg, Operand(addr_reg));
-  and_(bitmap_reg, ~Page::kPageAlignmentMask);
+  ASSERT(!AreAliased(addr_reg, mask_reg, bitmap_reg, ecx));
+  mov(bitmap_reg, Immediate(~Page::kPageAlignmentMask));
+  and_(Operand(bitmap_reg), addr_reg);
   mov(ecx, Operand(addr_reg));
   int shift =
       Bitmap::kBitsPerCellLog2 + kPointerSizeLog2 - Bitmap::kBytesPerCellLog2;
@@ -2412,11 +2389,65 @@ void MacroAssembler::EnsureNotWhite(
   }
 
   // Value is white.  We check whether it is data that doesn't need scanning.
-  JumpIfDataObject(value, ecx, value_is_white_and_not_data, distance);
+  // Currently only checks for HeapNumber and non-cons strings.
+  Register map = ecx;  // Holds map while checking type.
+  Register length = ecx;  // Holds length of object after checking type.
+  Label not_heap_number;
+  Label is_data_object;
+
+  // Check for heap-number
+  mov(map, FieldOperand(value, HeapObject::kMapOffset));
+  cmp(map, FACTORY->heap_number_map());
+  j(not_equal, &not_heap_number, Label::kNear);
+  mov(length, Immediate(HeapNumber::kSize));
+  jmp(&is_data_object, Label::kNear);
 
+  bind(&not_heap_number);
+  // Check for strings.
+  ASSERT(kConsStringTag == 1 && kIsConsStringMask == 1);
+  ASSERT(kNotStringTag == 0x80 && kIsNotStringMask == 0x80);
+  // If it's a string and it's not a cons string then it's an object containing
+  // no GC pointers.
+  Register instance_type = ecx;
+  movzx_b(instance_type, FieldOperand(map, Map::kInstanceTypeOffset));
+  test_b(Operand(instance_type), kIsConsStringMask | kIsNotStringMask);
+  j(not_zero, value_is_white_and_not_data);
+  // It's a non-cons string.
+  // If it's external, the length is just ExternalString::kSize.
+  // Otherwise it's String::kHeaderSize + string->length() * (1 or 2).
+  Label not_external;
+  test_b(Operand(instance_type), kExternalStringTag);

[Erik Corry, 2011/06/24 09:46:17]

You are making use of the fact that kExternalStringTag has only one bit set and
no other string tags have that tag set, so you should assert it.  I would also
consider not handling external strings here.  I don't think they are common
enough that it is worth the complexity.

[Lasse Reichstein, 2011/06/24 11:09:08]

Added assert.
Are we sure it's not more used in the browser?

+  j(zero, &not_external, Label::kNear);
+  mov(length, Immediate(ExternalString::kSize));
+  jmp(&is_data_object, Label::kNear);
+  bind(&not_external);

[Erik Corry, 2011/06/24 09:46:17]

I think a blank line before bind(...) lines improves clarity.

[Lasse Reichstein, 2011/06/24 11:09:08]

added

+  // Sequential string, either ASCII or UC16.
+  ASSERT(kAsciiStringTag == 0x04);
+  and_(Operand(length), Immediate(kStringEncodingMask));
+  xor_(Operand(length), Immediate(kStringEncodingMask));
+  add(Operand(length), Immediate(0x04));
+  // Value now either 4 (if ASCII) or 8 (if UC16), i.e., char-size shifted
+  // by 2. If we multiply the string length as smi by this, it still
+  // won't overflow a 32-bit value.
+  ASSERT_EQ(SeqAsciiString::kMaxSize, SeqAsciiString::kMaxSize);

[Erik Corry, 2011/06/24 09:46:17]

I have a hard time seeing the circumstances where this assert might fail.

[Lasse Reichstein, 2011/06/24 11:09:08]

*Cough*
Fixed.

+  ASSERT(SeqAsciiString::kMaxSize <=
+         static_cast<int>(0xffffffffu >> (2 + kSmiTagSize)));
+  imul(length, FieldOperand(value, String::kLengthOffset));
+  shr(length, 2 + kSmiTagSize);
+  add(Operand(length),
+      Immediate(SeqString::kHeaderSize + kObjectAlignmentMask));
+  and_(Operand(length),
+       Immediate(~kObjectAlignmentMask));
+
+  bind(&is_data_object);
   // Value is a data object, and it is white.  Mark it black.  Since we know
   // that the object is white we can make it black by flipping one bit.
   or_(Operand(bitmap_scratch, MemoryChunk::kHeaderSize), mask_scratch);
+
+  and_(bitmap_scratch, Immediate(~Page::kPageAlignmentMask));
+  add(Operand(bitmap_scratch, MemoryChunk::kLiveBytesOffset),

[Erik Corry, 2011/06/24 09:46:17]

I wonder how the performance of this read-modify-write instruction compares with
doing an add followed by a store?

[Lasse Reichstein, 2011/06/24 11:09:08]

Possibly a few cycle slower latency (but we're most likely not going to read it
in the next few cycles anyway), should be outweighed by the smaller code.

+      length);
+
   bind(&done);
 }