Reviewers: yurys,
Description:
When we do native memory snapshot we counts only external string resources
that
have been added to heap->external_string_table_.
I found that not all the strings that have external resources were added to
this
table.
We do not add Symbols to the table probably because they have different
lifetime.
But these symbols also could have external resources. As example it could
happen
when we assign innerHtml.
The simplest solution is to visit symbol_table too.
BUG=none
TEST=VisitExternalStrings
R=yurys
Please review this at https://chromiumcodereview.appspot.com/11315004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/heap.cc
M test/cctest/test-api.cc
Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index
909e7605b0f3094bec5a69472e8a38ed7eed4f5b..a51c790f636c084bd121dbb79005375b11022f0a
100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -1586,6 +1586,7 @@ void
Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) {
v8::ExternalResourceVisitor* visitor_;
} visitor_adapter(visitor);
external_string_table_.Iterate(&visitor_adapter);
+ symbol_table()->IterateElements(&visitor_adapter);
}
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
3be068009e275e53bb60903d812af6f841ab6069..fad9b7a2d3ccad4d60f46f18da7f96cd04cefc78
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -14816,11 +14816,13 @@ THREADED_TEST(GetHeapStatistics) {
class VisitorImpl : public v8::ExternalResourceVisitor {
public:
- VisitorImpl(TestResource* r1, TestResource* r2)
+ VisitorImpl(TestResource* r1, TestResource* r2, TestResource* r3)
: resource1_(r1),
resource2_(r2),
+ resource3_(r3),
found_resource1_(false),
- found_resource2_(false) {}
+ found_resource2_(false),
+ found_resource3_(false) {}
virtual ~VisitorImpl() {}
virtual void VisitExternalString(v8::Handle<v8::String> string) {
if (!string->IsExternal()) {
@@ -14838,17 +14840,24 @@ class VisitorImpl : public
v8::ExternalResourceVisitor {
CHECK(!found_resource2_);
found_resource2_ = true;
}
+ if (resource3_ == resource) {
+ CHECK(!found_resource3_);
+ found_resource3_ = true;
+ }
}
void CheckVisitedResources() {
CHECK(found_resource1_);
CHECK(found_resource2_);
+ CHECK(found_resource3_);
}
private:
v8::String::ExternalStringResource* resource1_;
v8::String::ExternalStringResource* resource2_;
+ v8::String::ExternalStringResource* resource3_;
bool found_resource1_;
bool found_resource2_;
+ bool found_resource3_;
};
TEST(VisitExternalStrings) {
@@ -14860,12 +14869,17 @@ TEST(VisitExternalStrings) {
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* resource3 = new TestResource(two_byte_string);
+ v8::Local<v8::String> symbol = v8::String::NewSymbol("a symbol string");
+ symbol->MakeExternal(resource3);
- // We need to add usages for string1 and string2 to avoid warnings in
GCC 4.7
+ // We need to add usages for string1, string2 and symbol string
+ // to avoid warnings in GCC 4.7
CHECK(string1->IsExternal());
CHECK(string2->IsExternal());
+ CHECK(symbol->IsExternal());
- VisitorImpl visitor(resource1, resource2);
+ VisitorImpl visitor(resource1, resource2, resource3);
v8::V8::VisitExternalResources(&visitor);
visitor.CheckVisitedResources();
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
