Add one-char string table for faster String.charAt to the VM.

runtime/vm/symbols.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/symbols.h"
 
 #include "vm/isolate.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
 #include "vm/raw_object.h"
@@ skipping 10 matching lines @@
 static const char* names[] = {
   NULL,
 
 #define DEFINE_SYMBOL_LITERAL(symbol, literal)                                 \
   literal,
 PREDEFINED_SYMBOLS_LIST(DEFINE_SYMBOL_LITERAL)
 #undef DEFINE_SYMBOL_LITERAL
 };
 
 
-const char* Symbols::Name(intptr_t symbol) {
-  ASSERT((symbol > kIllegal) && (symbol < kMaxId));
+const char* Symbols::Name(SymbolId symbol) {

[Ivan Posva, 2012/11/15 09:07:10]

I find it unfortunate that we cannot access the name over the full range of
valid SymbolIds.

[Florian Schneider, 2012/11/15 21:54:42]

Yes, but not sure if the names are actually needed for those >= kMaxPredefined.

+  ASSERT((symbol > kIllegal) && (symbol < kMaxPredefinedId));

[Ivan Posva, 2012/11/15 09:07:10]

I find it unfortunate that we cannot access the names over the full SymbolId
range.

   return names[symbol];
 }
 
 
 void Symbols::InitOnce(Isolate* isolate) {
   // Should only be run by the vm isolate.
   ASSERT(isolate == Dart::vm_isolate());
 
   // Create and setup a symbol table in the vm isolate.
   SetupSymbolTable(isolate);
 
   // Turn off population of symbols in the VM symbol table, so that we
   // don't find these symbols while doing a Symbols::New(...).
   // Create all predefined symbols.
-  ASSERT((sizeof(names) / sizeof(const char*)) == Symbols::kMaxId);
+  ASSERT((sizeof(names) / sizeof(const char*)) == Symbols::kMaxPredefinedId);
   const Array& symbol_table =
       Array::Handle(isolate->object_store()->symbol_table());
   dart::String& str = String::Handle();
 
-  for (intptr_t i = 1; i < Symbols::kMaxId; i++) {
+  for (intptr_t i = 1; i < Symbols::kMaxPredefinedId; i++) {

[Ivan Posva, 2012/11/15 09:07:10]

if the single char strings would be part of the names table, then you would not
need to have special handling here.

     str = OneByteString::New(names[i], Heap::kOld);
     Add(symbol_table, str);

[Ivan Posva, 2012/11/15 09:07:10]

The naked Add into the symbol table seems rather suspicious here. I know this is
not in your code, but we should verify that the symbol table never grows as we
keep it cached throughout this loop.

Fixed with https://chromiumcodereview.appspot.com/11308032/

     predefined_[i] = str.raw();
   }
   Object::RegisterSingletonClassNames();
+
+  for (uint32_t c = 0; c <= kMaxOneByteCharCode; c++) {
+    ASSERT(kMaxPredefinedId + c < kMaxId);
+    predefined_[kMaxPredefinedId + c] = New(&c, 1);
+  }
 }
 
 
 void Symbols::SetupSymbolTable(Isolate* isolate) {
   ASSERT(isolate != NULL);
 
   // Setup the symbol table used within the String class.
   const int initial_size = (isolate == Dart::vm_isolate()) ?
       kInitialVMIsolateSymtabSize : kInitialSymtabSize;
   const Array& array = Array::Handle(Array::New(initial_size + 1));
@@ skipping 43 matching lines @@
   ASSERT((type == Utf8::kBMP) || (type == Utf8::kSMP));
   uint16_t* characters = zone->Alloc<uint16_t>(len);
   Utf8::DecodeToUTF16(utf8_array, str_len, characters, len);
   return New(characters, len);
 }
 
 
 template<typename T>
 RawString* Symbols::New(const T* characters, intptr_t len) {
   Isolate* isolate = Isolate::Current();
-  ASSERT(isolate != Dart::vm_isolate());

[Ivan Posva, 2012/11/15 09:07:10]

If single char was handled in names, you would not need to remove this assertion
either.

[Florian Schneider, 2012/11/15 21:54:42]

It is a bit unfortunate, but I also want to avoid duplicates between the
predefined symbols and all one-char strings. It should not hurt to invoke
Symbols::New inside the vm-isolate, except that the code below will try twice to
find any non-existing string before adding it.

   String& symbol = String::Handle(isolate, String::null());
   Array& symbol_table = Array::Handle(isolate, Array::null());
 
   // Calculate the String hash for this sequence of characters.
   intptr_t hash = String::Hash(characters, len);
 
   // First check if a symbol exists in the vm isolate for these characters.
   symbol_table = Dart::vm_isolate()->object_store()->symbol_table();
   intptr_t index = FindIndex(symbol_table, characters, len, hash);
   symbol ^= symbol_table.At(index);
@@ skipping 62 matching lines @@
         symbol.SetHash(hash);
       }
       InsertIntoSymbolTable(symbol_table, symbol, index);
     }
   }
   ASSERT(symbol.IsSymbol());
   return symbol.raw();
 }
 
 
+RawString* Symbols::FromCharCode(uint32_t char_code) {
+  if (char_code > kMaxOneByteCharCode) {
+    return New(&char_code, 1);
+  }
+  return predefined_[kMaxPredefinedId + char_code];
+}
+
+
 void Symbols::GrowSymbolTable(const Array& symbol_table) {
   // TODO(iposva): Avoid exponential growth.
   intptr_t table_size = symbol_table.Length() - 1;
   intptr_t new_table_size = table_size * 2;
   Array& new_symbol_table = Array::Handle(Array::New(new_table_size + 1));
   // Copy all elements from the original symbol table to the newly allocated
   // array.
   String& element = String::Handle();
   dart::Object& new_element = Object::Handle();
   for (intptr_t i = 0; i < table_size; i++) {
@@ skipping 98 matching lines @@
 }
 
 
 RawObject* Symbols::GetVMSymbol(intptr_t object_id) {
   ASSERT(IsVMSymbolId(object_id));
   intptr_t i = (object_id - kMaxPredefinedObjectIds);
   return (i > 0 && i < Symbols::kMaxId) ? predefined_[i] : Object::null();
 }
 
 }  // namespace dart