StringTable::LookupKey() and all callers handlified.

R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/249103002

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

src/factory.cc

@@ -242,9 +242,8 @@ Handle<String> Factory::InternalizeUtf8String(Vector<const char> string) {
 
 // Internalized strings are created in the old generation (data space).
 Handle<String> Factory::InternalizeString(Handle<String> string) {
-  CALL_HEAP_FUNCTION(isolate(),
+  if (string->IsInternalizedString()) return string;
-                     isolate()->heap()->InternalizeString(*string),
+  return StringTable::LookupString(isolate(), string);
-                     String);
 }
 
 
@@ -269,9 +268,7 @@ Handle<String> Factory::InternalizeTwoByteString(Vector<const uc16> string) {
 
 template<class StringTableKey>
 Handle<String> Factory::InternalizeStringWithKey(StringTableKey* key) {
-  CALL_HEAP_FUNCTION(isolate(),
+  return StringTable::LookupKey(isolate(), key);
-                     isolate()->heap()->InternalizeStringWithKey(key),
-                     String);
 }
 
 
@@ -350,11 +347,27 @@ MaybeHandle<SeqTwoByteString> Factory::NewRawTwoByteString(
 }
 
 
-Handle<String> Factory::LookupSingleCharacterStringFromCode(uint32_t index) {
+Handle<String> Factory::LookupSingleCharacterStringFromCode(uint32_t code) {
-  CALL_HEAP_FUNCTION(
+  if (code <= String::kMaxOneByteCharCodeU) {
-      isolate(),
+    {
-      isolate()->heap()->LookupSingleCharacterStringFromCode(index),
+      DisallowHeapAllocation no_allocation;
-      String);
+      Object* value = single_character_string_cache()->get(code);
+      if (value != *undefined_value()) {
+        return handle(String::cast(value), isolate());
+      }
+    }
+    uint8_t buffer[1];
+    buffer[0] = static_cast<uint8_t>(code);
+    Handle<String> result =
+        InternalizeOneByteString(Vector<const uint8_t>(buffer, 1));
+    single_character_string_cache()->set(code, *result);
+    return result;
+  }
+  ASSERT(code <= String::kMaxUtf16CodeUnitU);
+
+  Handle<SeqTwoByteString> result = NewRawTwoByteString(1).ToHandleChecked();
+  result->SeqTwoByteStringSet(0, static_cast<uint16_t>(code));
+  return result;
 }
 
 

src/factory.h

@@ -81,6 +81,8 @@ class Factory V8_FINAL {
   // Create an empty TypeFeedbackInfo.
   Handle<TypeFeedbackInfo> NewTypeFeedbackInfo();
 
+  // Finds the internalized copy for string in the string table.
+  // If not found, a new string is added to the table and returned.
   Handle<String> InternalizeUtf8String(Vector<const char> str);
   Handle<String> InternalizeUtf8String(const char* str) {
     return InternalizeUtf8String(CStrVector(str));
@@ -165,7 +167,9 @@ class Factory V8_FINAL {
       int length,
       PretenureFlag pretenure = NOT_TENURED);
 
-  Handle<String> LookupSingleCharacterStringFromCode(uint32_t index);
+  // Creates a single character string where the character has given code.
+  // A cache is used for ASCII codes.
+  Handle<String> LookupSingleCharacterStringFromCode(uint32_t code);
 
   // Create a new cons string object which consists of a pair of strings.
   MUST_USE_RESULT MaybeHandle<String> NewConsString(Handle<String> left,
@@ -575,6 +579,10 @@ class Factory V8_FINAL {
   INTERNALIZED_STRING_LIST(STRING_ACCESSOR)
 #undef STRING_ACCESSOR
 
+  inline void set_string_table(Handle<StringTable> table) {
+    isolate()->heap()->set_string_table(*table);
+  }
+
   Handle<String> hidden_string() {
     return Handle<String>(&isolate()->heap()->hidden_string_);
   }

src/heap.cc

@@ -3387,31 +3387,6 @@ MaybeObject* Heap::AllocateForeign(Address address, PretenureFlag pretenure) {
 }
 
 
-MaybeObject* Heap::LookupSingleCharacterStringFromCode(uint16_t code) {
-  if (code <= String::kMaxOneByteCharCode) {
-    Object* value = single_character_string_cache()->get(code);
-    if (value != undefined_value()) return value;
-
-    uint8_t buffer[1];
-    buffer[0] = static_cast<uint8_t>(code);
-    Object* result;
-    OneByteStringKey key(Vector<const uint8_t>(buffer, 1), HashSeed());
-    MaybeObject* maybe_result = InternalizeStringWithKey(&key);
-
-    if (!maybe_result->ToObject(&result)) return maybe_result;
-    single_character_string_cache()->set(code, result);
-    return result;
-  }
-
-  SeqTwoByteString* result;
-  { MaybeObject* maybe_result = AllocateRawTwoByteString(1, NOT_TENURED);
-    if (!maybe_result->To<SeqTwoByteString>(&result)) return maybe_result;
-  }
-  result->SeqTwoByteStringSet(0, code);
-  return result;
-}
-
-
 MaybeObject* Heap::AllocateByteArray(int length, PretenureFlag pretenure) {
   if (length < 0 || length > ByteArray::kMaxLength) {
     v8::internal::Heap::FatalProcessOutOfMemory("invalid array length", true);
@@ -4875,28 +4850,6 @@ void Heap::Verify() {
 #endif
 
 
-MaybeObject* Heap::InternalizeUtf8String(Vector<const char> string) {
-  Utf8StringKey key(string, HashSeed());
-  return InternalizeStringWithKey(&key);
-}
-
-
-MaybeObject* Heap::InternalizeString(String* string) {
-  if (string->IsInternalizedString()) return string;
-  Object* result = NULL;
-  Object* new_table;
-  { MaybeObject* maybe_new_table =
-        string_table()->LookupString(string, &result);
-    if (!maybe_new_table->ToObject(&new_table)) return maybe_new_table;
-  }
-  // Can't use set_string_table because StringTable::cast knows that
-  // StringTable is a singleton and checks for identity.
-  roots_[kStringTableRootIndex] = new_table;
-  ASSERT(result != NULL);
-  return result;
-}
-
-
 bool Heap::InternalizeStringIfExists(String* string, String** result) {
   if (string->IsInternalizedString()) {
     *result = string;
@@ -4906,21 +4859,6 @@ bool Heap::InternalizeStringIfExists(String* string, String** result) {
 }
 
 
-MaybeObject* Heap::InternalizeStringWithKey(HashTableKey* key) {
-  Object* result = NULL;
-  Object* new_table;
-  { MaybeObject* maybe_new_table =
-        string_table()->LookupKey(key, &result);
-    if (!maybe_new_table->ToObject(&new_table)) return maybe_new_table;
-  }
-  // Can't use set_string_table because StringTable::cast knows that
-  // StringTable is a singleton and checks for identity.
-  roots_[kStringTableRootIndex] = new_table;
-  ASSERT(result != NULL);
-  return result;
-}
-
-
 void Heap::ZapFromSpace() {
   NewSpacePageIterator it(new_space_.FromSpaceStart(),
                           new_space_.FromSpaceEnd());

src/heap.h

@@ -828,13 +828,6 @@ class Heap {
   MUST_USE_RESULT MaybeObject* AllocateInternalizedStringImpl(
       T t, int chars, uint32_t hash_field);
 
-  // Computes a single character string where the character has code.
-  // A cache is used for ASCII codes.
-  // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation
-  // failed. Please note this does not perform a garbage collection.
-  MUST_USE_RESULT MaybeObject* LookupSingleCharacterStringFromCode(
-      uint16_t code);
-
   // Allocate a byte array of the specified length
   // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation
   // failed.
@@ -1005,12 +998,6 @@ class Heap {
   // Returns Failure::RetryAfterGC(requested_bytes, space) if allocation
   // failed.
   // Please note this function does not perform a garbage collection.
-  MUST_USE_RESULT MaybeObject* InternalizeUtf8String(const char* str) {
-    return InternalizeUtf8String(CStrVector(str));
-  }
-  MUST_USE_RESULT MaybeObject* InternalizeUtf8String(Vector<const char> str);
-
-  MUST_USE_RESULT MaybeObject* InternalizeString(String* str);
   MUST_USE_RESULT MaybeObject* InternalizeStringWithKey(HashTableKey* key);
 
   bool InternalizeStringIfExists(String* str, String** result);

src/mark-compact.cc

@@ -2146,7 +2146,10 @@ void MarkCompactCollector::MarkStringTable(RootMarkingVisitor* visitor) {
   StringTable* string_table = heap()->string_table();
   // Mark the string table itself.
   MarkBit string_table_mark = Marking::MarkBitFrom(string_table);
-  SetMark(string_table, string_table_mark);
+  if (!string_table_mark.Get()) {
+    // String table could have already been marked by visiting the handles list.
+    SetMark(string_table, string_table_mark);
+  }
   // Explicitly mark the prefix.
   string_table->IteratePrefix(visitor);
   ProcessMarkingDeque();

src/objects-inl.h

@@ -470,13 +470,18 @@ uc32 FlatStringReader::Get(int index) {
 }
 
 
+Handle<Object> HashTableKey::AsHandle(Isolate* isolate) {
+  CALL_HEAP_FUNCTION(isolate, AsObject(isolate->heap()), Object);
+}
+
+
 template <typename Char>
 class SequentialStringKey : public HashTableKey {
  public:
   explicit SequentialStringKey(Vector<const Char> string, uint32_t seed)
       : string_(string), hash_field_(0), seed_(seed) { }
 
-  virtual uint32_t Hash() {
+  virtual uint32_t Hash() V8_OVERRIDE {
     hash_field_ = StringHasher::HashSequentialString<Char>(string_.start(),
                                                            string_.length(),
                                                            seed_);
@@ -487,7 +492,7 @@ class SequentialStringKey : public HashTableKey {
   }
 
 
-  virtual uint32_t HashForObject(Object* other) {
+  virtual uint32_t HashForObject(Object* other) V8_OVERRIDE {
     return String::cast(other)->Hash();
   }
 
@@ -502,11 +507,11 @@ class OneByteStringKey : public SequentialStringKey<uint8_t> {
   OneByteStringKey(Vector<const uint8_t> str, uint32_t seed)
       : SequentialStringKey<uint8_t>(str, seed) { }
 
-  virtual bool IsMatch(Object* string) {
+  virtual bool IsMatch(Object* string) V8_OVERRIDE {
     return String::cast(string)->IsOneByteEqualTo(string_);
   }
 
-  virtual MaybeObject* AsObject(Heap* heap);
+  virtual MaybeObject* AsObject(Heap* heap) V8_OVERRIDE;
 };
 
 
@@ -521,7 +526,7 @@ class SubStringKey : public HashTableKey {
     ASSERT(string_->IsSeqString() || string->IsExternalString());
   }
 
-  virtual uint32_t Hash() {
+  virtual uint32_t Hash() V8_OVERRIDE {
     ASSERT(length_ >= 0);
     ASSERT(from_ + length_ <= string_->length());
     const Char* chars = GetChars() + from_;
@@ -532,12 +537,12 @@ class SubStringKey : public HashTableKey {
     return result;
   }
 
-  virtual uint32_t HashForObject(Object* other) {
+  virtual uint32_t HashForObject(Object* other) V8_OVERRIDE {
     return String::cast(other)->Hash();
   }
 
-  virtual bool IsMatch(Object* string);
+  virtual bool IsMatch(Object* string) V8_OVERRIDE;
-  virtual MaybeObject* AsObject(Heap* heap);
+  virtual MaybeObject* AsObject(Heap* heap) V8_OVERRIDE;
 
  private:
   const Char* GetChars();
@@ -562,11 +567,11 @@ class TwoByteStringKey : public SequentialStringKey<uc16> {
   explicit TwoByteStringKey(Vector<const uc16> str, uint32_t seed)
       : SequentialStringKey<uc16>(str, seed) { }
 
-  virtual bool IsMatch(Object* string) {
+  virtual bool IsMatch(Object* string) V8_OVERRIDE {
     return String::cast(string)->IsTwoByteEqualTo(string_);
   }
 
-  virtual MaybeObject* AsObject(Heap* heap);
+  virtual MaybeObject* AsObject(Heap* heap) V8_OVERRIDE;
 };
 
 
@@ -576,11 +581,11 @@ class Utf8StringKey : public HashTableKey {
   explicit Utf8StringKey(Vector<const char> string, uint32_t seed)
       : string_(string), hash_field_(0), seed_(seed) { }
 
-  virtual bool IsMatch(Object* string) {
+  virtual bool IsMatch(Object* string) V8_OVERRIDE {
     return String::cast(string)->IsUtf8EqualTo(string_);
   }
 
-  virtual uint32_t Hash() {
+  virtual uint32_t Hash() V8_OVERRIDE {
     if (hash_field_ != 0) return hash_field_ >> String::kHashShift;
     hash_field_ = StringHasher::ComputeUtf8Hash(string_, seed_, &chars_);
     uint32_t result = hash_field_ >> String::kHashShift;
@@ -588,11 +593,11 @@ class Utf8StringKey : public HashTableKey {
     return result;
   }
 
-  virtual uint32_t HashForObject(Object* other) {
+  virtual uint32_t HashForObject(Object* other) V8_OVERRIDE {
     return String::cast(other)->Hash();
   }
 
-  virtual MaybeObject* AsObject(Heap* heap) {
+  virtual MaybeObject* AsObject(Heap* heap) V8_OVERRIDE {
     if (hash_field_ == 0) Hash();
     return heap->AllocateInternalizedStringFromUtf8(string_,
                                                     chars_,

src/objects.cc

@@ -14536,37 +14536,37 @@ template class SubStringKey<uint16_t>;
 // InternalizedStringKey carries a string/internalized-string object as key.
 class InternalizedStringKey : public HashTableKey {
  public:
-  explicit InternalizedStringKey(String* string)
+  explicit InternalizedStringKey(Handle<String> string)
       : string_(string) { }
 
-  bool IsMatch(Object* string) {
+  virtual bool IsMatch(Object* string) V8_OVERRIDE {
-    return String::cast(string)->Equals(string_);
+    return String::cast(string)->Equals(*string_);
   }
 
-  uint32_t Hash() { return string_->Hash(); }
+  virtual uint32_t Hash() V8_OVERRIDE { return string_->Hash(); }
 
-  uint32_t HashForObject(Object* other) {
+  virtual uint32_t HashForObject(Object* other) V8_OVERRIDE {
     return String::cast(other)->Hash();
   }
 
-  MaybeObject* AsObject(Heap* heap) {
+  virtual MaybeObject* AsObject(Heap* heap) V8_OVERRIDE {
     // Internalize the string if possible.
-    Map* map = heap->InternalizedStringMapForString(string_);
+    Map* map = heap->InternalizedStringMapForString(*string_);
     if (map != NULL) {
       string_->set_map_no_write_barrier(map);
       ASSERT(string_->IsInternalizedString());
-      return string_;
+      return *string_;
     }
     // Otherwise allocate a new internalized string.
     return heap->AllocateInternalizedStringImpl(
-        string_, string_->length(), string_->hash_field());
+        *string_, string_->length(), string_->hash_field());
   }
 
   static uint32_t StringHash(Object* obj) {
     return String::cast(obj)->Hash();
   }
 
-  String* string_;
+  Handle<String> string_;
 };
 
 
@@ -15492,12 +15492,6 @@ Handle<PropertyCell> JSGlobalObject::EnsurePropertyCell(
 }
 
 
-MaybeObject* StringTable::LookupString(String* string, Object** s) {
-  InternalizedStringKey key(string);
-  return LookupKey(&key, s);
-}
-
-
 // This class is used for looking up two character strings in the string table.
 // If we don't have a hit we don't want to waste much time so we unroll the
 // string hash calculation loop here for speed.  Doesn't work if the two
@@ -15564,7 +15558,10 @@ class TwoCharHashTableKey : public HashTableKey {
 
 bool StringTable::LookupStringIfExists(String* string, String** result) {
   SLOW_ASSERT(this == HeapObject::cast(this)->GetHeap()->string_table());
-  InternalizedStringKey key(string);
+  DisallowHeapAllocation no_alloc;
+  // TODO(ishell): Handlify all the callers and remove this scope.
+  HandleScope scope(GetIsolate());
+  InternalizedStringKey key(handle(string));
   int entry = FindEntry(&key);
   if (entry == kNotFound) {
     return false;
@@ -15592,39 +15589,38 @@ bool StringTable::LookupTwoCharsStringIfExists(uint16_t c1,
 }
 
 
-MaybeObject* StringTable::LookupKey(HashTableKey* key, Object** s) {
+Handle<String> StringTable::LookupString(Isolate* isolate,
-  SLOW_ASSERT(this == HeapObject::cast(this)->GetHeap()->string_table());
+                                         Handle<String> string) {
-  int entry = FindEntry(key);
+  InternalizedStringKey key(string);
+  return LookupKey(isolate, &key);
+}
+
+
+// TODO(ishell): Maybehandlify callers.
+Handle<String> StringTable::LookupKey(Isolate* isolate, HashTableKey* key) {
+  Handle<StringTable> table = isolate->factory()->string_table();
+  int entry = table->FindEntry(key);
 
   // String already in table.
   if (entry != kNotFound) {
-    *s = KeyAt(entry);
+    return handle(String::cast(table->KeyAt(entry)), isolate);
-    return this;
   }
 
   // Adding new string. Grow table if needed.
-  Object* obj;
+  table = StringTable::EnsureCapacity(table, 1, key);
-  { MaybeObject* maybe_obj = EnsureCapacity(1, key);
-    if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-  }
 
   // Create string object.
-  Object* string;
+  Handle<Object> string = key->AsHandle(isolate);
-  { MaybeObject* maybe_string = key->AsObject(GetHeap());
+  // TODO(ishell): Maybehandlify this.
-    if (!maybe_string->ToObject(&string)) return maybe_string;
+  if (string.is_null()) return Handle<String>();
-  }
-
-  // If the string table grew as part of EnsureCapacity, obj is not
-  // the current string table and therefore we cannot use
-  // StringTable::cast here.
-  StringTable* table = reinterpret_cast<StringTable*>(obj);
 
   // Add the new string and return it along with the string table.
   entry = table->FindInsertionEntry(key->Hash());
-  table->set(EntryToIndex(entry), string);
+  table->set(EntryToIndex(entry), *string);
   table->ElementAdded();
-  *s = string;
+
-  return table;
+  isolate->factory()->set_string_table(table);
+  return Handle<String>::cast(string);
 }
 
 

src/objects.h

@@ -3881,6 +3881,8 @@ class HashTableKey {
   // Returns the key object for storing into the hash table.
   // If allocations fails a failure object is returned.
   MUST_USE_RESULT virtual MaybeObject* AsObject(Heap* heap) = 0;
+  // TODO(ishell): This should eventually replace AsObject().
+  inline Handle<Object> AsHandle(Isolate* isolate);
   // Required.
   virtual ~HashTableKey() {}
 };
@@ -3918,12 +3920,10 @@ class StringTable: public HashTable<StringTable,
                                     StringTableShape,
                                     HashTableKey*> {
  public:
-  // Find string in the string table.  If it is not there yet, it is
+  // Find string in the string table. If it is not there yet, it is
-  // added.  The return value is the string table which might have
+  // added. The return value is the string found.
-  // been enlarged.  If the return value is not a failure, the string
+  static Handle<String> LookupString(Isolate* isolate, Handle<String> key);
-  // pointer *s is set to the string found.
+  static Handle<String> LookupKey(Isolate* isolate, HashTableKey* key);
-  MUST_USE_RESULT MaybeObject* LookupString(String* key, Object** s);
-  MUST_USE_RESULT MaybeObject* LookupKey(HashTableKey* key, Object** s);
 
   // Looks up a string that is equal to the given string and returns
   // true if it is found, assigning the string to the given output
@@ -9217,6 +9217,7 @@ class String: public Name {
   static const int32_t kMaxOneByteCharCode = unibrow::Latin1::kMaxChar;
   static const uint32_t kMaxOneByteCharCodeU = unibrow::Latin1::kMaxChar;
   static const int kMaxUtf16CodeUnit = 0xffff;
+  static const uint32_t kMaxUtf16CodeUnitU = kMaxUtf16CodeUnit;
 
   // Value of hash field containing computed hash equal to zero.
   static const int kEmptyStringHash = kIsNotArrayIndexMask;

src/parser.cc

@@ -3842,8 +3842,7 @@ bool RegExpParser::simple() {
 
 RegExpTree* RegExpParser::ReportError(Vector<const char> message) {
   failed_ = true;
-  *error_ = isolate()->factory()->NewStringFromAscii(
+  *error_ = isolate()->factory()->NewStringFromAscii(message).ToHandleChecked();
-      message, NOT_TENURED).ToHandleChecked();
   // Zip to the end to make sure the no more input is read.
   current_ = kEndMarker;
   next_pos_ = in()->length();

test/cctest/test-api.cc

@@ -17993,7 +17993,8 @@ TEST(VisitExternalStrings) {
   CcTest::heap()->CollectAllAvailableGarbage();  // Tenure string.
   // Turn into a symbol.
   i::Handle<i::String> string3_i = v8::Utils::OpenHandle(*string3);
-  CHECK(!CcTest::heap()->InternalizeString(*string3_i)->IsFailure());
+  CHECK(!CcTest::i_isolate()->factory()->InternalizeString(
+      string3_i).is_null());
   CHECK(string3_i->IsInternalizedString());
 
   // We need to add usages for string* to avoid warnings in GCC 4.7

test/cctest/test-heap.cc

@@ -595,17 +595,13 @@ static const char* not_so_random_string_table[] = {
 
 
 static void CheckInternalizedStrings(const char** strings) {
+  Factory* factory = CcTest::i_isolate()->factory();
   for (const char* string = *strings; *strings != 0; string = *strings++) {
-    Object* a;
+    Handle<String> a = factory->InternalizeUtf8String(string);
-    MaybeObject* maybe_a = CcTest::heap()->InternalizeUtf8String(string);
-    // InternalizeUtf8String may return a failure if a GC is needed.
-    if (!maybe_a->ToObject(&a)) continue;
     CHECK(a->IsInternalizedString());
-    Object* b;
+    Handle<String> b = factory->InternalizeUtf8String(string);
-    MaybeObject* maybe_b = CcTest::heap()->InternalizeUtf8String(string);
+    CHECK_EQ(*b, *a);
-    if (!maybe_b->ToObject(&b)) continue;
+    CHECK(String::cast(*b)->IsUtf8EqualTo(CStrVector(string)));
-    CHECK_EQ(b, a);
-    CHECK(String::cast(b)->IsUtf8EqualTo(CStrVector(string)));
   }
 }
 
@@ -613,6 +609,7 @@ static void CheckInternalizedStrings(const char** strings) {
 TEST(StringTable) {
   CcTest::InitializeVM();
 
+  v8::HandleScope sc(CcTest::isolate());
   CheckInternalizedStrings(not_so_random_string_table);
   CheckInternalizedStrings(not_so_random_string_table);
 }