Do not shortcut cons string symbols during garbage collection.

Attempt to flatten cons strings when converting them to symbols so
that symbols will most often be flat strings.
Review URL: http://codereview.chromium.org/1700

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@253 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index b007b54..e155e77 100644 (file)
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -221,15 +221,15 @@ static MarkingStack marking_stack;
 
 
 inline HeapObject* ShortCircuitConsString(Object** p) {
-  // Optimization: If the heap object pointed to by p is a cons string whose
-  // right substring is Heap::empty_string, update it in place to its left
-  // substring.  Return the updated value.
+  // Optimization: If the heap object pointed to by p is a non-symbol
+  // cons string whose right substring is Heap::empty_string, update
+  // it in place to its left substring.  Return the updated value.
   //
   // Here we assume that if we change *p, we replace it with a heap object
   // (ie, the left substring of a cons string is always a heap object).
   //
   // The check performed is:
-  //   object->IsConsString() &&
+  //   object->IsConsString() && !object->IsSymbol() &&
   //   (ConsString::cast(object)->second() == Heap::empty_string())
   // except the maps for the object and its possible substrings might be
   // marked.
@@ -237,7 +237,9 @@ inline HeapObject* ShortCircuitConsString(Object** p) {
   MapWord map_word = object->map_word();
   map_word.ClearMark();
   InstanceType type = map_word.ToMap()->instance_type();
-  if (type >= FIRST_NONSTRING_TYPE) return object;
+  if (type >= FIRST_NONSTRING_TYPE || (type & kIsSymbolMask) != 0) {
+    return object;
+  }
 
   StringRepresentationTag rep =
       static_cast<StringRepresentationTag>(type & kStringRepresentationMask);

diff --git a/src/objects.cc b/src/objects.cc
index d87a5e2..3f29c71 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1124,8 +1124,7 @@ Object* JSObject::ReplaceConstantFunctionProperty(String* name,
   if (value->IsJSFunction()) {
     JSFunction* function = JSFunction::cast(value);
 
-    Object* new_map =
-      map()->CopyDropTransitions();
+    Object* new_map = map()->CopyDropTransitions();
     if (new_map->IsFailure()) return new_map;
     set_map(Map::cast(new_map));
 
@@ -2646,7 +2645,7 @@ Object* DescriptorArray::CopyInsert(Descriptor* descriptor,
   int new_size = number_of_descriptors() - transitions - null_descriptors;
 
   // If key is in descriptor, we replace it in-place when filtering.
-  int index = Search(descriptor->key());
+  int index = Search(descriptor->GetKey());
   const bool inserting = (index == kNotFound);
   const bool replacing = !inserting;
   bool keep_enumeration_index = false;
@@ -2689,7 +2688,7 @@ Object* DescriptorArray::CopyInsert(Descriptor* descriptor,
   // and inserting or replacing a descriptor.
   DescriptorWriter w(new_descriptors);
   DescriptorReader r(this);
-  uint32_t descriptor_hash = descriptor->key()->Hash();
+  uint32_t descriptor_hash = descriptor->GetKey()->Hash();
 
   for (; !r.eos(); r.advance()) {
     if (r.GetKey()->Hash() > descriptor_hash ||
@@ -5385,6 +5384,15 @@ class SymbolKey : public HashTableKey {
   uint32_t Hash() { return string_->Hash(); }
 
   Object* GetObject() {
+    // If the string is a cons string, attempt to flatten it so that
+    // symbols will most often be flat strings.
+    if (string_->IsConsString()) {
+      ConsString* cons_string = ConsString::cast(string_);
+      cons_string->TryFlatten();
+      if (cons_string->second() == Heap::empty_string()) {
+        string_ = String::cast(cons_string->first());
+      }
+    }
     // Transform string to symbol if possible.
     Map* map = Heap::SymbolMapForString(string_);
     if (map != NULL) {

diff --git a/src/parser.cc b/src/parser.cc
index 78a3273..5d7ed0f 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2556,7 +2556,7 @@ Expression* Parser::ParsePrimaryExpression(bool* ok) {
       Consume(Token::STRING);
       Handle<String> symbol =
           factory()->LookupSymbol(scanner_.literal_string(),
-                                scanner_.literal_length());
+                                  scanner_.literal_length());
       result = NEW(Literal(symbol));
       break;
     }
@@ -2708,8 +2708,8 @@ Expression* Parser::ParseObjectLiteral(bool* ok) {
       case Token::STRING: {
         Consume(Token::STRING);
         Handle<String> string =
-          factory()->LookupSymbol(scanner_.literal_string(),
-                                  scanner_.literal_length());
+            factory()->LookupSymbol(scanner_.literal_string(),
+                                    scanner_.literal_length());
         uint32_t index;
         if (!string.is_null() && string->AsArrayIndex(&index)) {
           key = NewNumberLiteral(index);
@@ -3032,7 +3032,7 @@ Handle<String> Parser::ParseIdentifier(bool* ok) {
   Expect(Token::IDENTIFIER, ok);
   if (!*ok) return Handle<String>();
   return factory()->LookupSymbol(scanner_.literal_string(),
-                               scanner_.literal_length());
+                                 scanner_.literal_length());
 }
 
 // This function reads an identifier and determines whether or not it

diff --git a/src/property.h b/src/property.h
index 1cd4cce..4b6de3a 100644 (file)
--- a/src/property.h
+++ b/src/property.h
@@ -44,8 +44,6 @@ class Descriptor BASE_EMBEDDED {
     return Smi::cast(value)->value();
   }
 
-  String* key() { return key_; }
-
   Object* KeyToSymbol() {
     if (!key_->IsSymbol()) {
       Object* result = Heap::LookupSymbol(key_);