Revision: 12841
Author: yang
...@chromium.org
Date: Fri Nov 2 05:45:00 2012
Log: Correctly visit all external strings.
BUG=
Review URL: https://chromiumcodereview.appspot.com/11340010
http://code.google.com/p/v8/source/detail?r=12841
Modified:
/branches/bleeding_edge/include/v8.h
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/include/v8.h Mon Oct 22 09:33:10 2012
+++ /branches/bleeding_edge/include/v8.h Fri Nov 2 05:45:00 2012
@@ -3444,8 +3444,8 @@
/**
* Iterates through all external resources referenced from current
isolate
- * heap. This method is not expected to be used except for debugging
purposes
- * and may be quite slow.
+ * heap. GC is not invoked prior to iterating, therefore there is no
+ * guarantee that visited objects are still alive.
*/
static void VisitExternalResources(ExternalResourceVisitor* visitor);
=======================================
--- /branches/bleeding_edge/src/heap.cc Fri Oct 26 02:44:34 2012
+++ /branches/bleeding_edge/src/heap.cc Fri Nov 2 05:45:00 2012
@@ -1576,13 +1576,40 @@
void Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) {
AssertNoAllocation no_allocation;
- class VisitorAdapter : public ObjectVisitor {
+ // Both the external string table and the symbol table may contain
+ // external strings, but neither lists them exhaustively, nor is the
+ // intersection set empty. Therefore we iterate over the external string
+ // table first, ignoring symbols, and then over the symbol table.
+
+ class ExternalStringTableVisitorAdapter : public ObjectVisitor {
public:
- explicit VisitorAdapter(v8::ExternalResourceVisitor* visitor)
- : visitor_(visitor) {}
+ explicit ExternalStringTableVisitorAdapter(
+ v8::ExternalResourceVisitor* visitor) : visitor_(visitor) {}
virtual void VisitPointers(Object** start, Object** end) {
for (Object** p = start; p < end; p++) {
+ // Visit non-symbol external strings,
+ // since symbols are listed in the symbol table.
+ if (!(*p)->IsSymbol()) {
+ ASSERT((*p)->IsExternalString());
+ visitor_->VisitExternalString(Utils::ToLocal(
+ Handle<String>(String::cast(*p))));
+ }
+ }
+ }
+ private:
+ v8::ExternalResourceVisitor* visitor_;
+ } external_string_table_visitor(visitor);
+
+ external_string_table_.Iterate(&external_string_table_visitor);
+
+ class SymbolTableVisitorAdapter : public ObjectVisitor {
+ public:
+ explicit SymbolTableVisitorAdapter(
+ v8::ExternalResourceVisitor* visitor) : visitor_(visitor) {}
+ virtual void VisitPointers(Object** start, Object** end) {
+ for (Object** p = start; p < end; p++) {
if ((*p)->IsExternalString()) {
+ ASSERT((*p)->IsSymbol());
visitor_->VisitExternalString(Utils::ToLocal(
Handle<String>(String::cast(*p))));
}
@@ -1590,8 +1617,9 @@
}
private:
v8::ExternalResourceVisitor* visitor_;
- } visitor_adapter(visitor);
- external_string_table_.Iterate(&visitor_adapter);
+ } symbol_table_visitor(visitor);
+
+ symbol_table()->IterateElements(&symbol_table_visitor);
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Oct 22 05:50:51 2012
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Nov 2 05:45:00 2012
@@ -14816,11 +14816,12 @@
class VisitorImpl : public v8::ExternalResourceVisitor {
public:
- VisitorImpl(TestResource* r1, TestResource* r2)
- : resource1_(r1),
- resource2_(r2),
- found_resource1_(false),
- found_resource2_(false) {}
+ explicit VisitorImpl(TestResource** resource) {
+ for (int i = 0; i < 4; i++) {
+ resource_[i] = resource[i];
+ found_resource_[i] = false;
+ }
+ }
virtual ~VisitorImpl() {}
virtual void VisitExternalString(v8::Handle<v8::String> string) {
if (!string->IsExternal()) {
@@ -14830,25 +14831,22 @@
v8::String::ExternalStringResource* resource =
string->GetExternalStringResource();
CHECK(resource);
- if (resource1_ == resource) {
- CHECK(!found_resource1_);
- found_resource1_ = true;
- }
- if (resource2_ == resource) {
- CHECK(!found_resource2_);
- found_resource2_ = true;
+ for (int i = 0; i < 4; i++) {
+ if (resource_[i] == resource) {
+ CHECK(!found_resource_[i]);
+ found_resource_[i] = true;
+ }
}
}
void CheckVisitedResources() {
- CHECK(found_resource1_);
- CHECK(found_resource2_);
+ for (int i = 0; i < 4; i++) {
+ CHECK(found_resource_[i]);
+ }
}
private:
- v8::String::ExternalStringResource* resource1_;
- v8::String::ExternalStringResource* resource2_;
- bool found_resource1_;
- bool found_resource2_;
+ v8::String::ExternalStringResource* resource_[4];
+ bool found_resource_[4];
};
TEST(VisitExternalStrings) {
@@ -14856,16 +14854,33 @@
LocalContext env;
const char* string = "Some string";
uint16_t* two_byte_string = AsciiToTwoByteString(string);
- TestResource* resource1 = new TestResource(two_byte_string);
- v8::Local<v8::String> string1 = v8::String::NewExternal(resource1);
- TestResource* resource2 = new TestResource(two_byte_string);
- v8::Local<v8::String> string2 = v8::String::NewExternal(resource2);
+ TestResource* resource[4];
+ resource[0] = new TestResource(two_byte_string);
+ v8::Local<v8::String> string0 = v8::String::NewExternal(resource[0]);
+ resource[1] = new TestResource(two_byte_string);
+ v8::Local<v8::String> string1 = v8::String::NewExternal(resource[1]);
- // We need to add usages for string1 and string2 to avoid warnings in
GCC 4.7
+ // Externalized symbol.
+ resource[2] = new TestResource(two_byte_string);
+ v8::Local<v8::String> string2 = v8::String::NewSymbol(string);
+ CHECK(string2->MakeExternal(resource[2]));
+
+ // Symbolized External.
+ resource[3] = new TestResource(AsciiToTwoByteString("Some other
string"));
+ v8::Local<v8::String> string3 = v8::String::NewExternal(resource[3]);
+ HEAP->CollectAllAvailableGarbage(); // Tenure string.
+ // Turn into a symbol.
+ i::Handle<i::String> string3_i = v8::Utils::OpenHandle(*string3);
+ CHECK(!HEAP->LookupSymbol(*string3_i)->IsFailure());
+ CHECK(string3_i->IsSymbol());
+
+ // We need to add usages for string* to avoid warnings in GCC 4.7
+ CHECK(string0->IsExternal());
CHECK(string1->IsExternal());
CHECK(string2->IsExternal());
+ CHECK(string3->IsExternal());
- VisitorImpl visitor(resource1, resource2);
+ VisitorImpl visitor(resource);
v8::V8::VisitExternalResources(&visitor);
visitor.CheckVisitedResources();
}
