[vm] Only include predefined symbols C strings in gen_snapshot.

Saves about 29k of binary size and kNumPredefinedSymbols map lookups at startup.

TEST=ci
Change-Id: I186c17a839ebfc6a800a35f08ca2f2c0776984ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/433862
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/bin/dartutils.cc

@@ -218,11 +218,6 @@ bool DartUtils::IsDartSchemeURL(const char* url_name) {
   return (strncmp(url_name, kDartScheme, kDartSchemeLen) == 0);
 }
 
-bool DartUtils::IsHttpSchemeURL(const char* url_name) {
-  static const intptr_t kHttpSchemeLen = strlen(kHttpScheme);
-  return (strncmp(url_name, kHttpScheme, kHttpSchemeLen) == 0);
-}
-
 bool DartUtils::IsDartIOLibURL(const char* url_name) {
   return (strcmp(url_name, kIOLibURL) == 0);
 }
@@ -239,15 +234,6 @@ bool DartUtils::IsDartBuiltinLibURL(const char* url_name) {
   return (strcmp(url_name, kBuiltinLibURL) == 0);
 }
 
-const char* DartUtils::RemoveScheme(const char* url) {
-  const char* colon = strchr(url, ':');
-  if (colon == nullptr) {
-    return url;
-  } else {
-    return colon + 1;
-  }
-}
-
 char* DartUtils::DirName(const char* url) {
   const char* slash = strrchr(url, File::PathSeparator()[0]);
   if (slash == nullptr) {

runtime/bin/dartutils.h

@@ -161,8 +161,6 @@ class DartUtils {
   static bool IsDartCLILibURL(const char* url_name);
   static bool IsDartHttpLibURL(const char* url_name);
   static bool IsDartBuiltinLibURL(const char* url_name);
-  static bool IsHttpSchemeURL(const char* url_name);
-  static const char* RemoveScheme(const char* url);
 
   // Returns directory name including the last path separator.
   //

runtime/vm/app_snapshot.cc

@@ -3718,9 +3718,6 @@ class RODataDeserializationCluster
       VerifyCanonicalSet(d, refs,
                          WeakArray::Handle(object_store->symbol_table()));
       object_store->set_symbol_table(table_);
-      if (d->isolate_group() == Dart::vm_isolate_group()) {
-        Symbols::InitFromSnapshot(d->isolate_group());
-      }
     } else if (!is_root_unit_ && is_canonical()) {
       FATAL("Cannot recanonicalize RO objects.");
     }
@@ -6790,9 +6787,6 @@ class StringDeserializationCluster
       VerifyCanonicalSet(d, refs,
                          WeakArray::Handle(object_store->symbol_table()));
       object_store->set_symbol_table(table_);
-      if (d->isolate_group() == Dart::vm_isolate_group()) {
-        Symbols::InitFromSnapshot(d->isolate_group());
-      }
 #if defined(DEBUG)
       Symbols::New(Thread::Current(), ":some:new:symbol:");
       ASSERT(object_store->symbol_table() == table_.ptr());  // Did not rehash.
@@ -6824,10 +6818,10 @@ class FakeSerializationCluster : public SerializationCluster {
 #if !defined(DART_PRECOMPILED_RUNTIME)
 class VMSerializationRoots : public SerializationRoots {
  public:
-  explicit VMSerializationRoots(const WeakArray& symbols,
-                                bool should_write_symbols)
-      : symbols_(symbols),
-        should_write_symbols_(should_write_symbols),
+  explicit VMSerializationRoots(const WeakArray& symbol_table,
+                                bool should_write_symbol_table)
+      : symbol_table_(symbol_table),
+        should_write_symbol_table_(should_write_symbol_table),
         zone_(Thread::Current()->zone()) {}
 
   void AddBaseObjects(Serializer* s) {
@@ -6901,11 +6895,11 @@ class VMSerializationRoots : public SerializationRoots {
   }
 
   void PushRoots(Serializer* s) {
-    if (should_write_symbols_) {
-      s->Push(symbols_.ptr());
+    if (should_write_symbol_table_) {
+      s->Push(symbol_table_.ptr());
     } else {
-      for (intptr_t i = 0; i < symbols_.Length(); i++) {
-        s->Push(symbols_.At(i));
+      for (intptr_t i = 0; i < symbol_table_.Length(); i++) {
+        s->Push(symbol_table_.At(i));
       }
     }
     if (Snapshot::IncludesCode(s->kind())) {
@@ -6916,35 +6910,41 @@ class VMSerializationRoots : public SerializationRoots {
   }
 
   void WriteRoots(Serializer* s) {
-    s->WriteRootRef(should_write_symbols_ ? symbols_.ptr() : Object::null(),
-                    "symbol-table");
+    for (intptr_t i = 1; i < Symbols::kMaxPredefinedId; i++) {
+      s->WriteRootRef(Symbols::Symbol(i).ptr(), "<symbol>");
+    }
+    s->WriteRootRef(
+        should_write_symbol_table_ ? symbol_table_.ptr() : Object::null(),
+        "symbol-table");
     if (Snapshot::IncludesCode(s->kind())) {
       for (intptr_t i = 0; i < StubCode::NumEntries(); i++) {
         s->WriteRootRef(StubCode::EntryAt(i).ptr(),
                         zone_->PrintToString("Stub:%s", StubCode::NameAt(i)));
       }
     }
 
-    if (!should_write_symbols_ && s->profile_writer() != nullptr) {
+    if (!should_write_symbol_table_ && s->profile_writer() != nullptr) {
       // If writing V8 snapshot profile create an artificial node representing
       // VM isolate symbol table.
-      ASSERT(!s->IsReachable(symbols_.ptr()));
-      s->AssignArtificialRef(symbols_.ptr());
-      const auto& symbols_snapshot_id = s->GetProfileId(symbols_.ptr());
-      s->profile_writer()->SetObjectTypeAndName(symbols_snapshot_id, "Symbols",
-                                                "vm_symbols");
-      s->profile_writer()->AddRoot(symbols_snapshot_id);
-      for (intptr_t i = 0; i < symbols_.Length(); i++) {
+      ASSERT(!s->IsReachable(symbol_table_.ptr()));
+      s->AssignArtificialRef(symbol_table_.ptr());
+      const auto& symbol_table_snapshot_id =
+          s->GetProfileId(symbol_table_.ptr());
+      s->profile_writer()->SetObjectTypeAndName(symbol_table_snapshot_id,
+                                                "Symbols", "vm_symbols");
+      s->profile_writer()->AddRoot(symbol_table_snapshot_id);
+      for (intptr_t i = 0; i < symbol_table_.Length(); i++) {
         s->profile_writer()->AttributeReferenceTo(
-            symbols_snapshot_id, V8SnapshotProfileWriter::Reference::Element(i),
-            s->GetProfileId(symbols_.At(i)));
+            symbol_table_snapshot_id,
+            V8SnapshotProfileWriter::Reference::Element(i),
+            s->GetProfileId(symbol_table_.At(i)));
       }
     }
   }
 
  private:
-  const WeakArray& symbols_;
-  const bool should_write_symbols_;
+  const WeakArray& symbol_table_;
+  const bool should_write_symbol_table_;
   Zone* zone_;
 };
 #endif  // !DART_PRECOMPILED_RUNTIME
@@ -7007,10 +7007,16 @@ class VMDeserializationRoots : public DeserializationRoots {
   }
 
   void ReadRoots(Deserializer* d) override {
+    for (intptr_t i = 1; i < Symbols::kMaxPredefinedId; i++) {
+      String* symbol = String::ReadOnlyHandle();
+      *symbol ^= d->ReadRef();
+      Symbols::InitSymbol(i, symbol);
+    }
     symbol_table_ ^= d->ReadRef();
     if (!symbol_table_.IsNull()) {
       d->isolate_group()->object_store()->set_symbol_table(symbol_table_);
     }
+    Symbols::InitFromSnapshot(d->isolate_group());
     if (Snapshot::IncludesCode(d->kind())) {
       for (intptr_t i = 0; i < StubCode::NumEntries(); i++) {
         Code* code = Code::ReadOnlyHandle();
@@ -7026,10 +7032,6 @@ class VMDeserializationRoots : public DeserializationRoots {
     // allocations (e.g., FinalizeVMIsolate) before allocating new pages.
     d->heap()->old_space()->ReleaseBumpAllocation();
 
-    if (!symbol_table_.IsNull()) {
-      Symbols::InitFromSnapshot(d->isolate_group());
-    }
-
     Object::set_vm_isolate_snapshot_object_table(refs);
   }
 
@@ -9701,7 +9703,7 @@ ZoneGrowableArray<Object*>* FullSnapshotWriter::WriteVMSnapshot() {
   VMSerializationRoots roots(
       WeakArray::Handle(
           Dart::vm_isolate_group()->object_store()->symbol_table()),
-      /*should_write_symbols=*/!Snapshot::IncludesStringsInROData(kind_));
+      /*should_write_symbol_table=*/!Snapshot::IncludesStringsInROData(kind_));
   ZoneGrowableArray<Object*>* objects = serializer.Serialize(&roots);
   serializer.FillHeader(serializer.kind());
   clustered_vm_size_ = serializer.bytes_written();

runtime/vm/symbols.cc

@@ -21,6 +21,7 @@ namespace dart {
 StringPtr Symbols::predefined_[Symbols::kNumberOfOneCharCodeSymbols];
 String* Symbols::symbol_handles_[Symbols::kMaxPredefinedId];
 
+#if !defined(DART_PRECOMPILED_RUNTIME)
 static const char* const names[] = {
     // clang-format off
   nullptr,
@@ -34,6 +35,7 @@ static const char* const names[] = {
 #undef DEFINE_TOKEN_SYMBOL_INDEX
     // clang-format on
 };
+#endif
 
 StringPtr StringFrom(const uint8_t* data, intptr_t len, Heap::Space space) {
   return String::FromLatin1(data, len, space);
@@ -63,11 +65,6 @@ StringPtr ConcatString::ToSymbol() const {
   return result.ptr();
 }
 
-const char* Symbols::Name(SymbolId symbol) {
-  ASSERT((symbol > kIllegal) && (symbol < kNullCharId));
-  return names[symbol];
-}
-
 const String& Symbols::Token(Token::Kind token) {
   const int tok_index = token;
   ASSERT((0 <= tok_index) && (tok_index < Token::kNumTokens));
@@ -78,6 +75,10 @@ const String& Symbols::Token(Token::Kind token) {
 }
 
 void Symbols::Init(IsolateGroup* vm_isolate_group) {
+  // TODO(engine): Require a snapshot when running the JIT runtime too.
+#if defined(DART_PRECOMPILED_RUNTIME)
+  UNREACHABLE();
+#else
   // Should only be run by the vm isolate.
   ASSERT(IsolateGroup::Current() == Dart::vm_isolate_group());
   ASSERT(vm_isolate_group == Dart::vm_isolate_group());
@@ -122,47 +123,14 @@ void Symbols::Init(IsolateGroup* vm_isolate_group) {
   }
 
   vm_isolate_group->object_store()->set_symbol_table(table.Release());
+#endif
 }
 
 void Symbols::InitFromSnapshot(IsolateGroup* vm_isolate_group) {
-  // Should only be run by the vm isolate.
-  ASSERT(IsolateGroup::Current() == Dart::vm_isolate_group());
-  ASSERT(vm_isolate_group == Dart::vm_isolate_group());
-  Zone* zone = Thread::Current()->zone();
-
-  CanonicalStringSet table(zone,
-                           vm_isolate_group->object_store()->symbol_table());
-
-  // Lookup all the predefined string symbols and language keyword symbols
-  // and cache them in the read only handles for fast access.
-  for (intptr_t i = 1; i < Symbols::kNullCharId; i++) {
-    String* str = String::ReadOnlyHandle();
-    const unsigned char* name =
-        reinterpret_cast<const unsigned char*>(names[i]);
-    *str ^= table.GetOrNull(Latin1Array(name, strlen(names[i])));
-    ASSERT(!str->IsNull());
-    ASSERT(str->HasHash());
-    ASSERT(str->IsCanonical());
-    symbol_handles_[i] = str;
-  }
-
-  // Lookup Latin1 character Symbols and cache them in read only handles,
-  // so that Symbols::FromCharCode is fast.
   for (intptr_t c = 0; c < kNumberOfOneCharCodeSymbols; c++) {
     intptr_t idx = (kNullCharId + c);
-    ASSERT(idx < kMaxPredefinedId);
-    ASSERT(Utf::IsLatin1(c));
-    uint8_t ch = static_cast<uint8_t>(c);
-    String* str = String::ReadOnlyHandle();
-    *str ^= table.GetOrNull(Latin1Array(&ch, 1));
-    ASSERT(!str->IsNull());
-    ASSERT(str->HasHash());
-    ASSERT(str->IsCanonical());
-    predefined_[c] = str->ptr();
-    symbol_handles_[idx] = str;
+    predefined_[c] = symbol_handles_[idx]->ptr();
   }
-
-  vm_isolate_group->object_store()->set_symbol_table(table.Release());
 }
 
 void Symbols::SetupSymbolTable(IsolateGroup* isolate_group) {

runtime/vm/symbols.h

@@ -622,6 +622,10 @@ class Symbols : public AllStatic {
     ASSERT((index > kIllegal) && (index < kMaxPredefinedId));
     return *(symbol_handles_[index]);
   }
+  static void InitSymbol(intptr_t index, String* symbol) {
+    ASSERT((index > kIllegal) && (index < kMaxPredefinedId));
+    symbol_handles_[index] = symbol;
+  }
 
   // Access methods for one byte character symbols stored in the vm isolate.
   static const String& Dot() { return *(symbol_handles_[kNullCharId + '.']); }
@@ -778,9 +782,6 @@ class Symbols : public AllStatic {
   static StringPtr FromSet(Thread* thread, const String& str);
   static StringPtr FromDot(Thread* thread, const String& str);
 
-  // Returns char* of predefined symbol.
-  static const char* Name(SymbolId symbol);
-
   static StringPtr FromCharCode(Thread* thread, uint16_t char_code);
 
   static StringPtr* PredefinedAddress() {