From 255b63ef504bea546874a598d8ee8dbb9363322a Mon Sep 17 00:00:00 2001 From: "ager@chromium.org" Date: Wed, 10 Sep 2008 11:35:05 +0000 Subject: [PATCH] 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 --- src/mark-compact.cc | 12 +++++++----- src/objects.cc | 16 ++++++++++++---- src/parser.cc | 8 ++++---- src/property.h | 2 -- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/mark-compact.cc b/src/mark-compact.cc index b007b5453..e155e779d 100644 --- 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(type & kStringRepresentationMask); diff --git a/src/objects.cc b/src/objects.cc index d87a5e227..3f29c71e9 100644 --- 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 78a32739c..5d7ed0f3a 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2556,7 +2556,7 @@ Expression* Parser::ParsePrimaryExpression(bool* ok) { Consume(Token::STRING); Handle 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 = - 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 Parser::ParseIdentifier(bool* ok) { Expect(Token::IDENTIFIER, ok); if (!*ok) return Handle(); 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 1cd4cce3e..4b6de3a98 100644 --- 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_); -- 2.34.1