Revision: 19093
Author: [email protected]
Date: Wed Feb 5 09:29:04 2014 UTC
Log: Allow externalizing strings in old pointer space.
This is what I think is a better solution to the "external strings in
old pointer space" problem. Basically, it is an issue because GC scans
all fields of objects in old pointer space and if the cached address
of the backing store is unaligned, it looks like a heap object, boom.
The solution here is to use short external strings when we externalize
a string in old pointer space, and when the address is unaligned.
Short external strings don't cache the address, so GC has no issues.
BUG=268686
LOG=Y
[email protected]
Review URL: https://codereview.chromium.org/146183006
http://code.google.com/p/v8/source/detail?r=19093
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/heap-inl.h
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Mon Jan 27 09:37:02 2014 UTC
+++ /branches/bleeding_edge/src/api.cc Wed Feb 5 09:29:04 2014 UTC
@@ -5424,23 +5424,6 @@
v8::String::ExternalAsciiStringResource* resource) {
return isolate->factory()->NewExternalStringFromAscii(resource);
}
-
-
-static bool RedirectToExternalString(i::Isolate* isolate,
- i::Handle<i::String> parent,
- i::Handle<i::String> external) {
- if (parent->IsConsString()) {
- i::Handle<i::ConsString> cons = i::Handle<i::ConsString>::cast(parent);
- cons->set_first(*external);
- cons->set_second(isolate->heap()->empty_string());
- } else {
- ASSERT(parent->IsSlicedString());
- i::Handle<i::SlicedString> slice =
i::Handle<i::SlicedString>::cast(parent);
- slice->set_parent(*external);
- slice->set_offset(0);
- }
- return true;
-}
Local<String> v8::String::NewExternal(
@@ -5472,22 +5455,10 @@
}
CHECK(resource && resource->data());
- bool result;
- i::Handle<i::String> external;
- if (isolate->heap()->old_pointer_space()->Contains(*obj)) {
- // We do not allow external strings in the old pointer space. Instead
of
- // converting the string in-place, we keep the cons/sliced string and
- // point it to a newly-allocated external string.
- external = NewExternalStringHandle(isolate, resource);
- result = RedirectToExternalString(isolate, obj, external);
- } else {
- result = obj->MakeExternal(resource);
- external = obj;
- }
-
+ bool result = obj->MakeExternal(resource);
if (result) {
- ASSERT(external->IsExternalString());
- isolate->heap()->external_string_table()->AddString(*external);
+ ASSERT(obj->IsExternalString());
+ isolate->heap()->external_string_table()->AddString(*obj);
}
return result;
}
@@ -5524,22 +5495,10 @@
}
CHECK(resource && resource->data());
- bool result;
- i::Handle<i::String> external;
- if (isolate->heap()->old_pointer_space()->Contains(*obj)) {
- // We do not allow external strings in the old pointer space. Instead
of
- // converting the string in-place, we keep the cons/sliced string and
- // point it to a newly-allocated external string.
- external = NewExternalAsciiStringHandle(isolate, resource);
- result = RedirectToExternalString(isolate, obj, external);
- } else {
- result = obj->MakeExternal(resource);
- external = obj;
- }
-
+ bool result = obj->MakeExternal(resource);
if (result) {
- ASSERT(external->IsExternalString());
- isolate->heap()->external_string_table()->AddString(*external);
+ ASSERT(obj->IsExternalString());
+ isolate->heap()->external_string_table()->AddString(*obj);
}
return result;
}
=======================================
--- /branches/bleeding_edge/src/heap-inl.h Tue Feb 4 10:30:36 2014 UTC
+++ /branches/bleeding_edge/src/heap-inl.h Wed Feb 5 09:29:04 2014 UTC
@@ -418,7 +418,7 @@
}
-bool Heap::AllowedToBeMigrated(HeapObject* object, AllocationSpace dst) {
+bool Heap::AllowedToBeMigrated(HeapObject* obj, AllocationSpace dst) {
// Object migration is governed by the following rules:
//
// 1) Objects in new-space can be migrated to one of the old spaces
@@ -428,18 +428,22 @@
// fixed arrays in new-space, old-data-space and old-pointer-space.
// 4) Fillers (one word) can never migrate, they are skipped by
// incremental marking explicitly to prevent invalid pattern.
+ // 5) Short external strings can end up in old pointer space when a cons
+ // string in old pointer space is made external
(String::MakeExternal).
//
// Since this function is used for debugging only, we do not place
// asserts here, but check everything explicitly.
- if (object->map() == one_pointer_filler_map()) return false;
- InstanceType type = object->map()->instance_type();
- MemoryChunk* chunk = MemoryChunk::FromAddress(object->address());
+ if (obj->map() == one_pointer_filler_map()) return false;
+ InstanceType type = obj->map()->instance_type();
+ MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address());
AllocationSpace src = chunk->owner()->identity();
switch (src) {
case NEW_SPACE:
return dst == src || dst == TargetSpaceId(type);
case OLD_POINTER_SPACE:
- return dst == src && (dst == TargetSpaceId(type) ||
object->IsFiller());
+ return dst == src &&
+ (dst == TargetSpaceId(type) || obj->IsFiller() ||
+ (obj->IsExternalString() &&
ExternalString::cast(obj)->is_short()));
case OLD_DATA_SPACE:
return dst == src && dst == TargetSpaceId(type);
case CODE_SPACE:
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Tue Feb 4 10:48:49 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h Wed Feb 5 09:29:04 2014 UTC
@@ -3193,6 +3193,7 @@
void ExternalAsciiString::set_resource(
const ExternalAsciiString::Resource* resource) {
+ ASSERT(IsAligned(reinterpret_cast<intptr_t>(resource), kPointerSize));
*reinterpret_cast<const Resource**>(
FIELD_ADDR(this, kResourceOffset)) = resource;
if (resource != NULL) update_data_cache();
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Feb 4 12:44:15 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Wed Feb 5 09:29:04 2014 UTC
@@ -1273,27 +1273,37 @@
bool is_ascii = this->IsOneByteRepresentation();
bool is_internalized = this->IsInternalizedString();
- // Morph the object to an external string by adjusting the map and
- // reinitializing the fields.
- if (size >= ExternalString::kSize) {
+ // Morph the string to an external string by replacing the map and
+ // reinitializing the fields. This won't work if
+ // - the space the existing string occupies is too small for a regular
+ // external string.
+ // - the existing string is in old pointer space and the backing store of
+ // the external string is not aligned. The GC cannot deal with fields
+ // containing an unaligned address that points to outside of V8's heap.
+ // In either case we resort to a short external string instead, omitting
+ // the field caching the address of the backing store. When we encounter
+ // short external strings in generated code, we need to bailout to
runtime.
+ if (size < ExternalString::kSize ||
+ (!IsAligned(reinterpret_cast<intptr_t>(resource->data()),
kPointerSize) &&
+ heap->old_pointer_space()->Contains(this))) {
this->set_map_no_write_barrier(
is_internalized
? (is_ascii
- ?
heap->external_internalized_string_with_one_byte_data_map()
- : heap->external_internalized_string_map())
+ ? heap->
+
short_external_internalized_string_with_one_byte_data_map()
+ : heap->short_external_internalized_string_map())
: (is_ascii
- ? heap->external_string_with_one_byte_data_map()
- : heap->external_string_map()));
+ ? heap->short_external_string_with_one_byte_data_map()
+ : heap->short_external_string_map()));
} else {
this->set_map_no_write_barrier(
is_internalized
- ? (is_ascii
- ? heap->
-
short_external_internalized_string_with_one_byte_data_map()
- : heap->short_external_internalized_string_map())
- : (is_ascii
- ? heap->short_external_string_with_one_byte_data_map()
- : heap->short_external_string_map()));
+ ? (is_ascii
+ ?
heap->external_internalized_string_with_one_byte_data_map()
+ : heap->external_internalized_string_map())
+ : (is_ascii
+ ? heap->external_string_with_one_byte_data_map()
+ : heap->external_string_map()));
}
ExternalTwoByteString* self = ExternalTwoByteString::cast(this);
self->set_resource(resource);
@@ -1334,16 +1344,26 @@
}
bool is_internalized = this->IsInternalizedString();
- // Morph the object to an external string by adjusting the map and
- // reinitializing the fields. Use short version if space is limited.
- if (size >= ExternalString::kSize) {
- this->set_map_no_write_barrier(
- is_internalized ? heap->external_ascii_internalized_string_map()
- : heap->external_ascii_string_map());
- } else {
+ // Morph the string to an external string by replacing the map and
+ // reinitializing the fields. This won't work if
+ // - the space the existing string occupies is too small for a regular
+ // external string.
+ // - the existing string is in old pointer space and the backing store of
+ // the external string is not aligned. The GC cannot deal with fields
+ // containing an unaligned address that points to outside of V8's heap.
+ // In either case we resort to a short external string instead, omitting
+ // the field caching the address of the backing store. When we encounter
+ // short external strings in generated code, we need to bailout to
runtime.
+ if (size < ExternalString::kSize ||
+ (!IsAligned(reinterpret_cast<intptr_t>(resource->data()),
kPointerSize) &&
+ heap->old_pointer_space()->Contains(this))) {
this->set_map_no_write_barrier(
is_internalized ?
heap->short_external_ascii_internalized_string_map()
: heap->short_external_ascii_string_map());
+ } else {
+ this->set_map_no_write_barrier(
+ is_internalized ? heap->external_ascii_internalized_string_map()
+ : heap->external_ascii_string_map());
}
ExternalAsciiString* self = ExternalAsciiString::cast(this);
self->set_resource(resource);
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue Feb 4 12:23:30
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Wed Feb 5 09:29:04
2014 UTC
@@ -17862,6 +17862,50 @@
};
+TEST(ExternalizeOldSpaceTwoByteCons) {
+ LocalContext env;
+ v8::HandleScope scope(env->GetIsolate());
+ v8::Local<v8::String> cons =
+ CompileRun("'Romeo Montague ' + 'Juliet Capulet'")->ToString();
+ CHECK(v8::Utils::OpenHandle(*cons)->IsConsString());
+ CcTest::heap()->CollectAllAvailableGarbage();
+ CHECK(CcTest::heap()->old_pointer_space()->Contains(
+ *v8::Utils::OpenHandle(*cons)));
+
+ TestResource* resource = new TestResource(
+ AsciiToTwoByteString("Romeo Montague Juliet Capulet"));
+ cons->MakeExternal(resource);
+
+ CHECK(cons->IsExternal());
+ CHECK_EQ(resource, cons->GetExternalStringResource());
+ String::Encoding encoding;
+ CHECK_EQ(resource, cons->GetExternalStringResourceBase(&encoding));
+ CHECK_EQ(String::TWO_BYTE_ENCODING, encoding);
+}
+
+
+TEST(ExternalizeOldSpaceOneByteCons) {
+ LocalContext env;
+ v8::HandleScope scope(env->GetIsolate());
+ v8::Local<v8::String> cons =
+ CompileRun("'Romeo Montague ' + 'Juliet Capulet'")->ToString();
+ CHECK(v8::Utils::OpenHandle(*cons)->IsConsString());
+ CcTest::heap()->CollectAllAvailableGarbage();
+ CHECK(CcTest::heap()->old_pointer_space()->Contains(
+ *v8::Utils::OpenHandle(*cons)));
+
+ TestAsciiResource* resource =
+ new TestAsciiResource(i::StrDup("Romeo Montague Juliet Capulet"));
+ cons->MakeExternal(resource);
+
+ CHECK(cons->IsExternalAscii());
+ CHECK_EQ(resource, cons->GetExternalAsciiStringResource());
+ String::Encoding encoding;
+ CHECK_EQ(resource, cons->GetExternalStringResourceBase(&encoding));
+ CHECK_EQ(String::ONE_BYTE_ENCODING, encoding);
+}
+
+
TEST(VisitExternalStrings) {
LocalContext env;
v8::HandleScope scope(env->GetIsolate());
--
--
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.
