Revision: 18285
Author: [email protected]
Date: Tue Dec 10 12:11:45 2013 UTC
Log: Track *all* external strings in the external string table.
Up till now, external strings may be tracked in the string table
(for internalized strings) or the external string table, depending
on in which order internalize and externalize happened.
The internalized string table should not have to deal with external
strings, all of which should be tracked by the external string table.
[email protected]
BUG=326489
LOG=N
Review URL: https://codereview.chromium.org/103663006
http://code.google.com/p/v8/source/detail?r=18285
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/heap.h
/branches/bleeding_edge/src/mark-compact.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Fri Dec 6 09:52:40 2013 UTC
+++ /branches/bleeding_edge/src/api.cc Tue Dec 10 12:11:45 2013 UTC
@@ -5733,8 +5733,8 @@
external = obj;
}
- ASSERT(external->IsExternalString());
- if (result && !external->IsInternalizedString()) {
+ if (result) {
+ ASSERT(external->IsExternalString());
isolate->heap()->external_string_table()->AddString(*external);
}
return result;
@@ -5791,8 +5791,8 @@
external = obj;
}
- ASSERT(external->IsExternalString());
- if (result && !external->IsInternalizedString()) {
+ if (result) {
+ ASSERT(external->IsExternalString());
isolate->heap()->external_string_table()->AddString(*external);
}
return result;
=======================================
--- /branches/bleeding_edge/src/heap.cc Fri Dec 6 09:52:40 2013 UTC
+++ /branches/bleeding_edge/src/heap.cc Tue Dec 10 12:11:45 2013 UTC
@@ -1938,12 +1938,7 @@
void Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) {
DisallowHeapAllocation no_allocation;
-
- // Both the external string table and the string 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 internalized strings, and then over the
- // internalized string table.
+ // All external strings are listed in the external string table.
class ExternalStringTableVisitorAdapter : public ObjectVisitor {
public:
@@ -1951,13 +1946,9 @@
v8::ExternalResourceVisitor* visitor) : visitor_(visitor) {}
virtual void VisitPointers(Object** start, Object** end) {
for (Object** p = start; p < end; p++) {
- // Visit non-internalized external strings,
- // since internalized strings are listed in the string table.
- if (!(*p)->IsInternalizedString()) {
- ASSERT((*p)->IsExternalString());
- visitor_->VisitExternalString(Utils::ToLocal(
- Handle<String>(String::cast(*p))));
- }
+ ASSERT((*p)->IsExternalString());
+ visitor_->VisitExternalString(Utils::ToLocal(
+ Handle<String>(String::cast(*p))));
}
}
private:
@@ -1965,25 +1956,6 @@
} external_string_table_visitor(visitor);
external_string_table_.Iterate(&external_string_table_visitor);
-
- class StringTableVisitorAdapter : public ObjectVisitor {
- public:
- explicit StringTableVisitorAdapter(
- v8::ExternalResourceVisitor* visitor) : visitor_(visitor) {}
- virtual void VisitPointers(Object** start, Object** end) {
- for (Object** p = start; p < end; p++) {
- if ((*p)->IsExternalString()) {
- ASSERT((*p)->IsInternalizedString());
- visitor_->VisitExternalString(Utils::ToLocal(
- Handle<String>(String::cast(*p))));
- }
- }
- }
- private:
- v8::ExternalResourceVisitor* visitor_;
- } string_table_visitor(visitor);
-
- string_table()->IterateElements(&string_table_visitor);
}
=======================================
--- /branches/bleeding_edge/src/heap.h Wed Dec 4 11:39:24 2013 UTC
+++ /branches/bleeding_edge/src/heap.h Tue Dec 10 12:11:45 2013 UTC
@@ -1416,14 +1416,6 @@
// Print short heap statistics.
void PrintShortHeapStatistics();
- // Makes a new internalized string object
- // Returns Failure::RetryAfterGC(requested_bytes, space) if the
allocation
- // failed.
- // Please note this function does not perform a garbage collection.
- MUST_USE_RESULT MaybeObject* CreateInternalizedString(
- const char* str, int length, int hash);
- MUST_USE_RESULT MaybeObject* CreateInternalizedString(String* str);
-
// Write barrier support for address[offset] = o.
INLINE(void RecordWrite(Address address, int offset));
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Fri Dec 6 09:52:40 2013 UTC
+++ /branches/bleeding_edge/src/mark-compact.cc Tue Dec 10 12:11:45 2013 UTC
@@ -1839,6 +1839,7 @@
// Helper class for pruning the string table.
+template<bool finalize_external_strings>
class StringTableCleaner : public ObjectVisitor {
public:
explicit StringTableCleaner(Heap* heap)
@@ -1850,22 +1851,20 @@
Object* o = *p;
if (o->IsHeapObject() &&
!Marking::MarkBitFrom(HeapObject::cast(o)).Get()) {
- // Check if the internalized string being pruned is external. We
need to
- // delete the associated external data as this string is going
away.
-
- // Since no objects have yet been moved we can safely access the
map of
- // the object.
- if (o->IsExternalString()) {
+ if (finalize_external_strings) {
+ ASSERT(o->IsExternalString());
heap_->FinalizeExternalString(String::cast(*p));
+ } else {
+ pointers_removed_++;
}
// Set the entry to the_hole_value (as deleted).
*p = heap_->the_hole_value();
- pointers_removed_++;
}
}
}
int PointersRemoved() {
+ ASSERT(!finalize_external_strings);
return pointers_removed_;
}
@@ -1875,6 +1874,10 @@
};
+typedef StringTableCleaner<false> InternalizedStringTableCleaner;
+typedef StringTableCleaner<true> ExternalStringTableCleaner;
+
+
// Implementation of WeakObjectRetainer for mark compact GCs. All marked
objects
// are retained.
class MarkCompactWeakObjectRetainer : public WeakObjectRetainer {
@@ -2398,10 +2401,12 @@
// string table. Cannot use string_table() here because the string
// table is marked.
StringTable* string_table = heap()->string_table();
- StringTableCleaner v(heap());
- string_table->IterateElements(&v);
- string_table->ElementsRemoved(v.PointersRemoved());
- heap()->external_string_table_.Iterate(&v);
+ InternalizedStringTableCleaner internalized_visitor(heap());
+ string_table->IterateElements(&internalized_visitor);
+ string_table->ElementsRemoved(internalized_visitor.PointersRemoved());
+
+ ExternalStringTableCleaner external_visitor(heap());
+ heap()->external_string_table_.Iterate(&external_visitor);
heap()->external_string_table_.CleanUp();
// Process the weak references.
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Dec 9 07:41:20
2013 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Tue Dec 10 12:11:45
2013 UTC
@@ -17340,6 +17340,55 @@
// Ring has been destroyed. Free Peoples of Middle-earth Rejoice.
CHECK_EQ(1, destroyed);
}
+
+
+TEST(ExternalInternalizedStringCollectedAtTearDown) {
+ int destroyed = 0;
+ v8::Isolate* isolate = v8::Isolate::New();
+ { v8::Isolate::Scope isolate_scope(isolate);
+ LocalContext env(isolate);
+ v8::HandleScope handle_scope(isolate);
+ CompileRun("var ring = 'One string to test them all';");
+ const char* s = "One string to test them all";
+ TestAsciiResource* inscription =
+ new TestAsciiResource(i::StrDup(s), &destroyed);
+ v8::Local<v8::String> ring = CompileRun("ring")->ToString();
+ CHECK(v8::Utils::OpenHandle(*ring)->IsInternalizedString());
+ ring->MakeExternal(inscription);
+ // Ring is still alive. Orcs are roaming freely across our lands.
+ CHECK_EQ(0, destroyed);
+ USE(ring);
+ }
+
+ isolate->Dispose();
+ // Ring has been destroyed. Free Peoples of Middle-earth Rejoice.
+ CHECK_EQ(1, destroyed);
+}
+
+
+TEST(ExternalInternalizedStringCollectedAtGC) {
+ int destroyed = 0;
+ { LocalContext env;
+ v8::HandleScope handle_scope(env->GetIsolate());
+ CompileRun("var ring = 'One string to test them all';");
+ const char* s = "One string to test them all";
+ TestAsciiResource* inscription =
+ new TestAsciiResource(i::StrDup(s), &destroyed);
+ v8::Local<v8::String> ring = CompileRun("ring")->ToString();
+ CHECK(v8::Utils::OpenHandle(*ring)->IsInternalizedString());
+ ring->MakeExternal(inscription);
+ // Ring is still alive. Orcs are roaming freely across our lands.
+ CHECK_EQ(0, destroyed);
+ USE(ring);
+ }
+
+ // Garbage collector deals swift blows to evil.
+ CcTest::i_isolate()->compilation_cache()->Clear();
+ CcTest::heap()->CollectAllAvailableGarbage();
+
+ // Ring has been destroyed. Free Peoples of Middle-earth Rejoice.
+ CHECK_EQ(1, destroyed);
+}
static double DoubleFromBits(uint64_t value) {
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
