External string table.

Instead of weak handles external strings use a separate table.  This
table uses 5 times less memory than weak handles.  Moreover, since we
don't have to follow the weak handle callback protocol we can collect
the strings faster and even on scavenge collections.

Review URL: http://codereview.chromium.org/467037

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

include/v8.h

@@ -833,13 +833,26 @@ class V8EXPORT String : public Primitive {
    * Returns true if the string is both external and ascii
    */
   bool IsExternalAscii() const;
+
+  class V8EXPORT ExternalStringResourceBase {
+   public:
+    virtual ~ExternalStringResourceBase() {}
+   protected:
+    ExternalStringResourceBase() {}
+   private:
+    // Disallow copying and assigning.
+    ExternalStringResourceBase(const ExternalStringResourceBase&);
+    void operator=(const ExternalStringResourceBase&);
+  };
+
   /**
    * An ExternalStringResource is a wrapper around a two-byte string
    * buffer that resides outside V8's heap. Implement an
    * ExternalStringResource to manage the life cycle of the underlying
    * buffer.  Note that the string data must be immutable.
    */
-  class V8EXPORT ExternalStringResource {  // NOLINT
+  class V8EXPORT ExternalStringResource
+      : public ExternalStringResourceBase {
    public:
     /**
      * Override the destructor to manage the life cycle of the underlying
@@ -852,10 +865,6 @@ class V8EXPORT String : public Primitive {
     virtual size_t length() const = 0;
    protected:
     ExternalStringResource() {}
-   private:
-    // Disallow copying and assigning.
-    ExternalStringResource(const ExternalStringResource&);
-    void operator=(const ExternalStringResource&);
   };
 
   /**
@@ -869,7 +878,8 @@ class V8EXPORT String : public Primitive {
    * Use String::New or convert to 16 bit data for non-ASCII.
    */
 
-  class V8EXPORT ExternalAsciiStringResource {  // NOLINT
+  class V8EXPORT ExternalAsciiStringResource
+      : public ExternalStringResourceBase {
    public:
     /**
      * Override the destructor to manage the life cycle of the underlying
@@ -882,10 +892,6 @@ class V8EXPORT String : public Primitive {
     virtual size_t length() const = 0;
    protected:
     ExternalAsciiStringResource() {}
-   private:
-    // Disallow copying and assigning.
-    ExternalAsciiStringResource(const ExternalAsciiStringResource&);
-    void operator=(const ExternalAsciiStringResource&);
   };
 
   /**

src/api.cc

@@ -3082,81 +3082,13 @@ i::Handle<i::String> NewExternalAsciiStringHandle(
 }
 
 
-static void DisposeExternalString(v8::Persistent<v8::Value> obj,
-                                  void* parameter) {
-  ENTER_V8;
-  i::ExternalTwoByteString* str =
-      i::ExternalTwoByteString::cast(*Utils::OpenHandle(*obj));
-
-  // External symbols are deleted when they are pruned out of the symbol
-  // table. Generally external symbols are not registered with the weak handle
-  // callbacks unless they are upgraded to a symbol after being externalized.
-  if (!str->IsSymbol()) {
-    v8::String::ExternalStringResource* resource =
-        reinterpret_cast<v8::String::ExternalStringResource*>(parameter);
-    if (resource != NULL) {
-      const int total_size =
-          static_cast<int>(resource->length() * sizeof(*resource->data()));
-      i::Counters::total_external_string_memory.Decrement(total_size);
-
-      // The object will continue to live in the JavaScript heap until the
-      // handle is entirely cleaned out by the next GC. For example the
-      // destructor for the resource below could bring it back to life again.
-      // Which is why we make sure to not have a dangling pointer here.
-      str->set_resource(NULL);
-      delete resource;
-    }
-  }
-
-  // In any case we do not need this handle any longer.
-  obj.Dispose();
-}
-
-
-static void DisposeExternalAsciiString(v8::Persistent<v8::Value> obj,
-                                       void* parameter) {
-  ENTER_V8;
-  i::ExternalAsciiString* str =
-      i::ExternalAsciiString::cast(*Utils::OpenHandle(*obj));
-
-  // External symbols are deleted when they are pruned out of the symbol
-  // table. Generally external symbols are not registered with the weak handle
-  // callbacks unless they are upgraded to a symbol after being externalized.
-  if (!str->IsSymbol()) {
-    v8::String::ExternalAsciiStringResource* resource =
-        reinterpret_cast<v8::String::ExternalAsciiStringResource*>(parameter);
-    if (resource != NULL) {
-      const int total_size =
-          static_cast<int>(resource->length() * sizeof(*resource->data()));
-      i::Counters::total_external_string_memory.Decrement(total_size);
-
-      // The object will continue to live in the JavaScript heap until the
-      // handle is entirely cleaned out by the next GC. For example the
-      // destructor for the resource below could bring it back to life again.
-      // Which is why we make sure to not have a dangling pointer here.
-      str->set_resource(NULL);
-      delete resource;
-    }
-  }
-
-  // In any case we do not need this handle any longer.
-  obj.Dispose();
-}
-
-
 Local<String> v8::String::NewExternal(
       v8::String::ExternalStringResource* resource) {
   EnsureInitialized("v8::String::NewExternal()");
   LOG_API("String::NewExternal");
   ENTER_V8;
-  const int total_size =
-      static_cast<int>(resource->length() * sizeof(*resource->data()));
-  i::Counters::total_external_string_memory.Increment(total_size);
   i::Handle<i::String> result = NewExternalStringHandle(resource);
-  i::Handle<i::Object> handle = i::GlobalHandles::Create(*result);
-  i::GlobalHandles::MakeWeak(handle.location(),
-                             resource,
-                             &DisposeExternalString);
+  i::ExternalStringTable::AddString(*result);
   return Utils::ToLocal(result);
 }
 
@@ -3168,13 +3100,7 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
   i::Handle<i::String> obj = Utils::OpenHandle(this);
   bool result = obj->MakeExternal(resource);
   if (result && !obj->IsSymbol()) {
-    // Operation was successful and the string is not a symbol. In this case
-    // we need to make sure that the we call the destructor for the external
-    // resource when no strong references to the string remain.
-    i::Handle<i::Object> handle = i::GlobalHandles::Create(*obj);
-    i::GlobalHandles::MakeWeak(handle.location(),
-                               resource,
-                               &DisposeExternalString);
+    i::ExternalStringTable::AddString(*obj);
   }
   return result;
 }
@@ -3185,14 +3111,8 @@ Local<String> v8::String::NewExternal(
   EnsureInitialized("v8::String::NewExternal()");
   LOG_API("String::NewExternal");
   ENTER_V8;
-  const int total_size =
-      static_cast<int>(resource->length() * sizeof(*resource->data()));
-  i::Counters::total_external_string_memory.Increment(total_size);
   i::Handle<i::String> result = NewExternalAsciiStringHandle(resource);
-  i::Handle<i::Object> handle = i::GlobalHandles::Create(*result);
-  i::GlobalHandles::MakeWeak(handle.location(),
-                             resource,
-                             &DisposeExternalAsciiString);
+  i::ExternalStringTable::AddString(*result);
   return Utils::ToLocal(result);
 }
 
@@ -3205,13 +3125,7 @@ bool v8::String::MakeExternal(
   i::Handle<i::String> obj = Utils::OpenHandle(this);
   bool result = obj->MakeExternal(resource);
   if (result && !obj->IsSymbol()) {
-    // Operation was successful and the string is not a symbol. In this case
-    // we need to make sure that the we call the destructor for the external
-    // resource when no strong references to the string remain.
-    i::Handle<i::Object> handle = i::GlobalHandles::Create(*obj);
-    i::GlobalHandles::MakeWeak(handle.location(),
-                               resource,
-                               &DisposeExternalAsciiString);
+    i::ExternalStringTable::AddString(*obj);
   }
   return result;
 }

src/global-handles.cc

@@ -168,6 +168,12 @@ class GlobalHandles::Node : public Malloced {
       if (first_deallocated()) {
         first_deallocated()->set_next(head());
       }
+      // Check that we are not passing a finalized external string to
+      // the callback.
+      ASSERT(!object_->IsExternalAsciiString() ||
+             ExternalAsciiString::cast(object_)->resource() != NULL);
+      ASSERT(!object_->IsExternalTwoByteString() ||
+             ExternalTwoByteString::cast(object_)->resource() != NULL);
       // Leaving V8.
       VMState state(EXTERNAL);
       func(object, par);
@@ -507,5 +513,4 @@ void GlobalHandles::RemoveObjectGroups() {
   object_groups->Clear();
 }
 
-
 } }  // namespace v8::internal

src/globals.h

@@ -294,7 +294,7 @@ enum GarbageCollector { SCAVENGER, MARK_COMPACTOR };
 
 enum Executability { NOT_EXECUTABLE, EXECUTABLE };
 
-enum VisitMode { VISIT_ALL, VISIT_ONLY_STRONG };
+enum VisitMode { VISIT_ALL, VISIT_ALL_IN_SCAVENGE, VISIT_ONLY_STRONG };
 
 
 // A CodeDesc describes a buffer holding instructions and relocation

src/heap-inl.h

@@ -109,6 +109,19 @@ Object* Heap::NumberFromUint32(uint32_t value) {
 }
 
 
+void Heap::FinalizeExternalString(String* string) {
+  ASSERT(string->IsExternalString());
+  v8::String::ExternalStringResourceBase** resource_addr =
+      reinterpret_cast<v8::String::ExternalStringResourceBase**>(
+          reinterpret_cast<byte*>(string) +
+          ExternalString::kResourceOffset -
+          kHeapObjectTag);
+  delete *resource_addr;
+  // Clear the resource pointer in the string.
+  *resource_addr = NULL;
+}
+
+
 Object* Heap::AllocateRawMap() {
 #ifdef DEBUG
   Counters::objs_since_last_full.Increment();
@@ -321,6 +334,56 @@ inline bool Heap::allow_allocation(bool new_state) {
 #endif
 
 
+void ExternalStringTable::AddString(String* string) {
+  ASSERT(string->IsExternalString());
+  if (Heap::InNewSpace(string)) {
+    new_space_strings_.Add(string);
+  } else {
+    old_space_strings_.Add(string);
+  }
+}
+
+
+void ExternalStringTable::Iterate(ObjectVisitor* v) {
+  if (!new_space_strings_.is_empty()) {
+    Object** start = &new_space_strings_[0];
+    v->VisitPointers(start, start + new_space_strings_.length());
+  }
+  if (!old_space_strings_.is_empty()) {
+    Object** start = &old_space_strings_[0];
+    v->VisitPointers(start, start + old_space_strings_.length());
+  }
+}
+
+
+// Verify() is inline to avoid ifdef-s around its calls in release
+// mode.
+void ExternalStringTable::Verify() {
+#ifdef DEBUG
+  for (int i = 0; i < new_space_strings_.length(); ++i) {
+    ASSERT(Heap::InNewSpace(new_space_strings_[i]));
+    ASSERT(new_space_strings_[i] != Heap::raw_unchecked_null_value());
+  }
+  for (int i = 0; i < old_space_strings_.length(); ++i) {
+    ASSERT(!Heap::InNewSpace(old_space_strings_[i]));
+    ASSERT(old_space_strings_[i] != Heap::raw_unchecked_null_value());
+  }
+#endif
+}
+
+
+void ExternalStringTable::AddOldString(String* string) {
+  ASSERT(string->IsExternalString());
+  ASSERT(!Heap::InNewSpace(string));
+  old_space_strings_.Add(string);
+}
+
+
+void ExternalStringTable::ShrinkNewStrings(int position) {
+  new_space_strings_.Rewind(position);
+  Verify();
+}
+
 } }  // namespace v8::internal
 
 #endif  // V8_HEAP_INL_H_

src/heap.cc

@@ -733,7 +733,7 @@ void Heap::Scavenge() {
 
   ScavengeVisitor scavenge_visitor;
   // Copy roots.
-  IterateRoots(&scavenge_visitor, VISIT_ALL);
+  IterateRoots(&scavenge_visitor, VISIT_ALL_IN_SCAVENGE);
 
   // Copy objects reachable from the old generation.  By definition,
   // there are no intergenerational pointers in code or data spaces.
@@ -753,6 +753,63 @@ void Heap::Scavenge() {
     }
   }
 
+  new_space_front = DoScavenge(&scavenge_visitor, new_space_front);
+
+  ScavengeExternalStringTable();
+  ASSERT(new_space_front == new_space_.top());
+
+  // Set age mark.
+  new_space_.set_age_mark(new_space_.top());
+
+  // Update how much has survived scavenge.
+  survived_since_last_expansion_ +=
+      (PromotedSpaceSize() - survived_watermark) + new_space_.Size();
+
+  LOG(ResourceEvent("scavenge", "end"));
+
+  gc_state_ = NOT_IN_GC;
+}
+
+
+void Heap::ScavengeExternalStringTable() {
+  ExternalStringTable::Verify();
+
+  if (ExternalStringTable::new_space_strings_.is_empty()) return;
+
+  Object** start = &ExternalStringTable::new_space_strings_[0];
+  Object** end = start + ExternalStringTable::new_space_strings_.length();
+  Object** last = start;
+
+  for (Object** p = start; p < end; ++p) {
+    ASSERT(Heap::InFromSpace(*p));
+    MapWord first_word = HeapObject::cast(*p)->map_word();
+
+    if (!first_word.IsForwardingAddress()) {
+      // Unreachable external string can be finalized.
+      FinalizeExternalString(String::cast(*p));
+      continue;
+    }
+
+    // String is still reachable.
+    String* target = String::cast(first_word.ToForwardingAddress());
+    ASSERT(target->IsExternalString());
+
+    if (Heap::InNewSpace(target)) {
+      // String is still in new space.  Update the table entry.
+      *last = target;
+      ++last;
+    } else {
+      // String got promoted.  Move it to the old string list.
+      ExternalStringTable::AddOldString(target);
+    }
+  }
+
+  ExternalStringTable::ShrinkNewStrings(last - start);
+}
+
+
+Address Heap::DoScavenge(ObjectVisitor* scavenge_visitor,
+                         Address new_space_front) {
   do {
     ASSERT(new_space_front <= new_space_.top());
 
@@ -761,7 +818,7 @@ void Heap::Scavenge() {
     // queue is empty.
     while (new_space_front < new_space_.top()) {
       HeapObject* object = HeapObject::FromAddress(new_space_front);
-      object->Iterate(&scavenge_visitor);
+      object->Iterate(scavenge_visitor);
       new_space_front += object->Size();
     }
 
@@ -783,7 +840,7 @@ void Heap::Scavenge() {
       RecordCopiedObject(target);
 #endif
       // Visit the newly copied object for pointers to new space.
-      target->Iterate(&scavenge_visitor);
+      target->Iterate(scavenge_visitor);
       UpdateRSet(target);
     }
 
@@ -791,16 +848,7 @@ void Heap::Scavenge() {
     // (there are currently no more unswept promoted objects).
   } while (new_space_front < new_space_.top());
 
-  // Set age mark.
-  new_space_.set_age_mark(new_space_.top());
-
-  // Update how much has survived scavenge.
-  survived_since_last_expansion_ +=
-      (PromotedSpaceSize() - survived_watermark) + new_space_.Size();
-
-  LOG(ResourceEvent("scavenge", "end"));
-
-  gc_state_ = NOT_IN_GC;
+  return new_space_front;
 }
 
 
@@ -3175,6 +3223,11 @@ void Heap::IterateRoots(ObjectVisitor* v, VisitMode mode) {
   IterateStrongRoots(v, mode);
   v->VisitPointer(reinterpret_cast<Object**>(&roots_[kSymbolTableRootIndex]));
   v->Synchronize("symbol_table");
+  if (mode != VISIT_ALL_IN_SCAVENGE) {
+    // Scavenge collections have special processing for this.
+    ExternalStringTable::Iterate(v);
+  }
+  v->Synchronize("external_string_table");
 }
 
 
@@ -3203,11 +3256,12 @@ void Heap::IterateStrongRoots(ObjectVisitor* v, VisitMode mode) {
   HandleScopeImplementer::Iterate(v);
   v->Synchronize("handlescope");
 
-  // Iterate over the builtin code objects and code stubs in the heap. Note
-  // that it is not strictly necessary to iterate over code objects on
-  // scavenge collections.  We still do it here because this same function
-  // is used by the mark-sweep collector and the deserializer.
-  Builtins::IterateBuiltins(v);
+  // Iterate over the builtin code objects and code stubs in the
+  // heap. Note that it is not necessary to iterate over code objects
+  // on scavenge collections.
+  if (mode != VISIT_ALL_IN_SCAVENGE) {
+    Builtins::IterateBuiltins(v);
+  }
   v->Synchronize("builtins");
 
   // Iterate over global handles.
@@ -3424,6 +3478,8 @@ void Heap::SetStackLimits() {
 void Heap::TearDown() {
   GlobalHandles::TearDown();
 
+  ExternalStringTable::TearDown();
+
   new_space_.TearDown();
 
   if (old_pointer_space_ != NULL) {
@@ -3839,8 +3895,8 @@ class MarkRootVisitor: public ObjectVisitor {
 
 // Triggers a depth-first traversal of reachable objects from roots
 // and finds a path to a specific heap object and prints it.
-void Heap::TracePathToObject() {
-  search_target = NULL;
+void Heap::TracePathToObject(Object* target) {
+  search_target = target;
   search_for_any_global = false;
 
   MarkRootVisitor root_visitor;
@@ -3991,4 +4047,35 @@ void TranscendentalCache::Clear() {
 }
 
 
+void ExternalStringTable::CleanUp() {
+  int last = 0;
+  for (int i = 0; i < new_space_strings_.length(); ++i) {
+    if (new_space_strings_[i] == Heap::raw_unchecked_null_value()) continue;
+    if (Heap::InNewSpace(new_space_strings_[i])) {
+      new_space_strings_[last++] = new_space_strings_[i];
+    } else {
+      old_space_strings_.Add(new_space_strings_[i]);
+    }
+  }
+  new_space_strings_.Rewind(last);
+  last = 0;
+  for (int i = 0; i < old_space_strings_.length(); ++i) {
+    if (old_space_strings_[i] == Heap::raw_unchecked_null_value()) continue;
+    ASSERT(!Heap::InNewSpace(old_space_strings_[i]));
+    old_space_strings_[last++] = old_space_strings_[i];
+  }
+  old_space_strings_.Rewind(last);
+  Verify();
+}
+
+
+void ExternalStringTable::TearDown() {
+  new_space_strings_.Free();
+  old_space_strings_.Free();
+}
+
+
+List<Object*> ExternalStringTable::new_space_strings_;
+List<Object*> ExternalStringTable::old_space_strings_;
+
 } }  // namespace v8::internal

src/heap.h

@@ -566,6 +566,10 @@ class Heap : public AllStatic {
   static Object* AllocateExternalStringFromTwoByte(
       ExternalTwoByteString::Resource* resource);
 
+  // Finalizes an external string by deleting the associated external
+  // data and clearing the resource pointer.
+  static inline void FinalizeExternalString(String* string);
+
   // Allocates an uninitialized object.  The memory is non-executable if the
   // hardware and OS allow.
   // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation
@@ -778,7 +782,7 @@ class Heap : public AllStatic {
     return disallow_allocation_failure_;
   }
 
-  static void TracePathToObject();
+  static void TracePathToObject(Object* target);
   static void TracePathToGlobal();
 #endif
 
@@ -1039,6 +1043,9 @@ class Heap : public AllStatic {
 
   // Performs a minor collection in new generation.
   static void Scavenge();
+  static void ScavengeExternalStringTable();
+  static Address DoScavenge(ObjectVisitor* scavenge_visitor,
+                            Address new_space_front);
 
   // Performs a major collection in the whole heap.
   static void MarkCompact(GCTracer* tracer);
@@ -1623,6 +1630,39 @@ class TranscendentalCache {
 };
 
 
+// External strings table is a place where all external strings are
+// registered.  We need to keep track of such strings to properly
+// finalize them.
+class ExternalStringTable : public AllStatic {
+ public:
+  // Registers an external string.
+  inline static void AddString(String* string);
+
+  inline static void Iterate(ObjectVisitor* v);
+
+  // Restores internal invariant and gets rid of collected strings.
+  // Must be called after each Iterate() that modified the strings.
+  static void CleanUp();
+
+  // Destroys all allocated memory.
+  static void TearDown();
+
+ private:
+  friend class Heap;
+
+  inline static void Verify();
+
+  inline static void AddOldString(String* string);
+
+  // Notifies the table that only a prefix of the new list is valid.
+  inline static void ShrinkNewStrings(int position);
+
+  // To speed up scavenge collections new space string are kept
+  // separate from old space strings.
+  static List<Object*> new_space_strings_;
+  static List<Object*> old_space_strings_;
+};
+
 } }  // namespace v8::internal
 
 #endif  // V8_HEAP_H_

src/mark-compact.cc

@@ -155,6 +155,8 @@ void MarkCompactCollector::Finish() {
   // objects (empty string, illegal builtin).
   StubCache::Clear();
 
+  ExternalStringTable::CleanUp();
+
   // If we've just compacted old space there's no reason to check the
   // fragmentation limit. Just return.
   if (HasCompacted()) return;
@@ -369,41 +371,18 @@ class RootMarkingVisitor : public ObjectVisitor {
 class SymbolTableCleaner : public ObjectVisitor {
  public:
   SymbolTableCleaner() : pointers_removed_(0) { }
-  void VisitPointers(Object** start, Object** end) {
+
+  virtual void VisitPointers(Object** start, Object** end) {
     // Visit all HeapObject pointers in [start, end).
     for (Object** p = start; p < end; p++) {
       if ((*p)->IsHeapObject() && !HeapObject::cast(*p)->IsMarked()) {
         // Check if the symbol being pruned is an external symbol. We need to
         // delete the associated external data as this symbol is going away.
 
-        // Since the object is not marked we can access its map word safely
-        // without having to worry about marking bits in the object header.
-        Map* map = HeapObject::cast(*p)->map();
         // Since no objects have yet been moved we can safely access the map of
         // the object.
-        uint32_t type = map->instance_type();
-        bool is_external = (type & kStringRepresentationMask) ==
-                           kExternalStringTag;
-        if (is_external) {
-          bool is_two_byte = (type & kStringEncodingMask) == kTwoByteStringTag;
-          byte* resource_addr = reinterpret_cast<byte*>(*p) +
-                                ExternalString::kResourceOffset -
-                                kHeapObjectTag;
-          if (is_two_byte) {
-            v8::String::ExternalStringResource** resource =
-                reinterpret_cast<v8::String::ExternalStringResource**>
-                (resource_addr);
-            delete *resource;
-            // Clear the resource pointer in the symbol.
-            *resource = NULL;
-          } else {
-            v8::String::ExternalAsciiStringResource** resource =
-                reinterpret_cast<v8::String::ExternalAsciiStringResource**>
-                (resource_addr);
-            delete *resource;
-            // Clear the resource pointer in the symbol.
-            *resource = NULL;
-          }
+        if ((*p)->IsExternalString()) {
+          Heap::FinalizeExternalString(String::cast(*p));
         }
         // Set the entry to null_value (as deleted).
         *p = Heap::raw_unchecked_null_value();
@@ -546,34 +525,7 @@ bool MarkCompactCollector::IsUnmarkedHeapObject(Object** p) {
 }
 
 
-class SymbolMarkingVisitor : public ObjectVisitor {
- public:
-  void VisitPointers(Object** start, Object** end) {
-    MarkingVisitor marker;
-    for (Object** p = start; p < end; p++) {
-      if (!(*p)->IsHeapObject()) continue;
-
-      HeapObject* object = HeapObject::cast(*p);
-      // If the object is marked, we have marked or are in the process
-      // of marking subparts.
-      if (object->IsMarked()) continue;
-
-      // The object is unmarked, we do not need to unmark to use its
-      // map.
-      Map* map = object->map();
-      object->IterateBody(map->instance_type(),
-                          object->SizeFromMap(map),
-                          &marker);
-    }
-  }
-};
-
-
 void MarkCompactCollector::MarkSymbolTable() {
-  // Objects reachable from symbols are marked as live so as to ensure
-  // that if the symbol itself remains alive after GC for any reason,
-  // and if it is a cons string backed by an external string (even indirectly),
-  // then the external string does not receive a weak reference callback.
   SymbolTable* symbol_table = Heap::raw_unchecked_symbol_table();
   // Mark the symbol table itself.
   SetMark(symbol_table);
@@ -581,11 +533,6 @@ void MarkCompactCollector::MarkSymbolTable() {
   MarkingVisitor marker;
   symbol_table->IteratePrefix(&marker);
   ProcessMarkingStack(&marker);
-  // Mark subparts of the symbols but not the symbols themselves
-  // (unless reachable from another symbol).
-  SymbolMarkingVisitor symbol_marker;
-  symbol_table->IterateElements(&symbol_marker);
-  ProcessMarkingStack(&marker);
 }
 
 
@@ -774,6 +721,8 @@ void MarkCompactCollector::MarkLiveObjects() {
   SymbolTableCleaner v;
   symbol_table->IterateElements(&v);
   symbol_table->ElementsRemoved(v.PointersRemoved());
+  ExternalStringTable::Iterate(&v);
+  ExternalStringTable::CleanUp();
 
   // Remove object groups after marking phase.
   GlobalHandles::RemoveObjectGroups();

src/v8-counters.h

@@ -74,8 +74,6 @@ namespace internal {
   SC(objs_since_last_full, V8.ObjsSinceLastFull)                 \
   SC(symbol_table_capacity, V8.SymbolTableCapacity)              \
   SC(number_of_symbols, V8.NumberOfSymbols)                      \
-  /* Current amount of memory in external string buffers. */     \
-  SC(total_external_string_memory, V8.TotalExternalStringMemory) \
   SC(script_wrappers, V8.ScriptWrappers)                         \
   SC(call_initialize_stubs, V8.CallInitializeStubs)              \
   SC(call_premonomorphic_stubs, V8.CallPreMonomorphicStubs)      \

test/cctest/test-api.cc

@@ -447,6 +447,40 @@ THREADED_TEST(UsingExternalAsciiString) {
 }
 
 
+THREADED_TEST(ScavengeExternalString) {
+  TestResource::dispose_count = 0;
+  {
+    v8::HandleScope scope;
+    uint16_t* two_byte_string = AsciiToTwoByteString("test string");
+    Local<String> string =
+        String::NewExternal(new TestResource(two_byte_string));
+    i::Handle<i::String> istring = v8::Utils::OpenHandle(*string);
+    i::Heap::CollectGarbage(0, i::NEW_SPACE);
+    CHECK(i::Heap::InNewSpace(*istring));
+    CHECK_EQ(0, TestResource::dispose_count);
+  }
+  i::Heap::CollectGarbage(0, i::NEW_SPACE);
+  CHECK_EQ(1, TestResource::dispose_count);
+}
+
+
+THREADED_TEST(ScavengeExternalAsciiString) {
+  TestAsciiResource::dispose_count = 0;
+  {
+    v8::HandleScope scope;
+    const char* one_byte_string = "test string";
+    Local<String> string = String::NewExternal(
+        new TestAsciiResource(i::StrDup(one_byte_string)));
+    i::Handle<i::String> istring = v8::Utils::OpenHandle(*string);
+    i::Heap::CollectGarbage(0, i::NEW_SPACE);
+    CHECK(i::Heap::InNewSpace(*istring));
+    CHECK_EQ(0, TestAsciiResource::dispose_count);
+  }
+  i::Heap::CollectGarbage(0, i::NEW_SPACE);
+  CHECK_EQ(1, TestAsciiResource::dispose_count);
+}
+
+
 THREADED_TEST(StringConcat) {
   {
     v8::HandleScope scope;