AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Speculatively Revert "Reland "Avoiding re-externalization of strings.""

Browse Source 
This reverts commit f34158c9d2.

Reason for revert: Seems to trigger DCHECKS. Two CLs in range; this one seemed more likely.
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064%20(dbg)/12787
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064%20(dbg)/12788


Original change's description:
> Reland "Avoiding re-externalization of strings."
> 
> This is a reland of 2c4c2ad694
> 
> Original change's description:
> > Avoiding re-externalization of strings.
> >
> > Bug: chromium:845409
> > Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
> > Change-Id: I75bddcf0e8879d2161486f24d1cd4e46d8fe008d
> > Reviewed-on: https://chromium-review.googlesource.com/1139056
> > Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#54599}
> 
> Bug: chromium:866208
> Change-Id: I7714bfc695ebeaf55b9ccbbc6b11368416ce7fec
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
> Reviewed-on: https://chromium-review.googlesource.com/1146583
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
> Cr-Commit-Position: refs/heads/master@{#54637}

TBR=ulan@chromium.org,jkummerow@chromium.org,hpayer@chromium.org,mlippautz@chromium.org,rfbpb@google.com

Change-Id: Id12382d66bc5c9b5c76d73b06a6b421dd4d7be66
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:866208
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1148400
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54645}
This commit is contained in:
Sigurd Schneider 2018-07-24 13:36:55 +00:00 committed by Commit Bot
parent 76db01d54e
commit 765c1eac1f
8 changed files with 47 additions and 171 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
include/v8.h
View File

@ -3048,12 +3048,6 @@ class V8_EXPORT String : public Name {
void VerifyExternalStringResourceBase(ExternalStringResourceBase* v,
Encoding encoding) const;
void VerifyExternalStringResource(ExternalStringResource* val) const;
ExternalStringResource* GetExternalStringResourceSlow() const;
ExternalStringResourceBase* GetExternalStringResourceBaseSlow(
String::Encoding* encoding_out) const;
const ExternalOneByteStringResource* GetExternalOneByteStringResourceSlow()
const;
static void CheckCast(v8::Value* obj);
};
@ -10188,13 +10182,12 @@ String::ExternalStringResource* String::GetExternalStringResource() const {
typedef internal::Object O;
typedef internal::Internals I;
O* obj = *reinterpret_cast<O* const*>(this);
ExternalStringResource* result;
String::ExternalStringResource* result;
if (I::IsExternalTwoByteString(I::GetInstanceType(obj))) {
void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
result = reinterpret_cast<String::ExternalStringResource*>(value);
} else {
result = GetExternalStringResourceSlow();
result = NULL;
}
#ifdef V8_ENABLE_CHECKS
VerifyExternalStringResource(result);
@ -10210,16 +10203,14 @@ String::ExternalStringResourceBase* String::GetExternalStringResourceBase(
O* obj = *reinterpret_cast<O* const*>(this);
int type = I::GetInstanceType(obj) & I::kFullStringRepresentationMask;
*encoding_out = static_cast<Encoding>(type & I::kStringEncodingMask);
ExternalStringResourceBase* resource;
ExternalStringResourceBase* resource = NULL;
if (type == I::kExternalOneByteRepresentationTag ||
type == I::kExternalTwoByteRepresentationTag) {
void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
resource = static_cast<ExternalStringResourceBase*>(value);
} else {
resource = GetExternalStringResourceBaseSlow(encoding_out);
}
#ifdef V8_ENABLE_CHECKS
VerifyExternalStringResourceBase(resource, *encoding_out);
VerifyExternalStringResourceBase(resource, *encoding_out);
#endif
return resource;
}

149
src/api.cc
View File

@ -5930,15 +5930,6 @@ void v8::String::VerifyExternalStringResource(
v8::String::ExternalStringResource* value) const {
i::Handle<i::String> str = Utils::OpenHandle(this);
const v8::String::ExternalStringResource* expected;
i::Isolate* isolate = nullptr;
i::Isolate::FromWritableHeapObject(*str, &isolate);
DCHECK(isolate != nullptr);
if (str->IsThinString()) {
str = i::Handle<i::String>(i::ThinString::cast(*str)->actual(), isolate);
}
if (i::StringShape(*str).IsExternalTwoByte()) {
const void* resource =
i::Handle<i::ExternalTwoByteString>::cast(str)->resource();
@ -5954,15 +5945,6 @@ void v8::String::VerifyExternalStringResourceBase(
i::Handle<i::String> str = Utils::OpenHandle(this);
const v8::String::ExternalStringResourceBase* expected;
Encoding expectedEncoding;
i::Isolate* isolate = nullptr;
i::Isolate::FromWritableHeapObject(*str, &isolate);
DCHECK(isolate != nullptr);
if (str->IsThinString()) {
str = i::Handle<i::String>(i::ThinString::cast(*str)->actual(), isolate);
}
if (i::StringShape(*str).IsExternalOneByte()) {
const void* resource =
i::Handle<i::ExternalOneByteString>::cast(str)->resource();
@ -5982,74 +5964,15 @@ void v8::String::VerifyExternalStringResourceBase(
CHECK_EQ(expectedEncoding, encoding);
}
String::ExternalStringResource* String::GetExternalStringResourceSlow() const {
typedef internal::Internals I;
i::Handle<i::String> str = Utils::OpenHandle(this);
ExternalStringResource* result = nullptr;
internal::Object* obj;
if (str->IsThinString()) {
obj = i::Handle<i::ThinString>::cast(str)->actual();
} else {
obj = *str;
}
if (I::IsExternalTwoByteString(I::GetInstanceType(obj))) {
void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
result = reinterpret_cast<String::ExternalStringResource*>(value);
}
return result;
}
String::ExternalStringResourceBase* String::GetExternalStringResourceBaseSlow(
String::Encoding* encoding_out) const {
typedef internal::Internals I;
i::Handle<i::String> str = Utils::OpenHandle(this);
ExternalStringResourceBase* resource = nullptr;
internal::Object* obj;
if (str->IsThinString()) {
obj = i::Handle<i::ThinString>::cast(str)->actual();
} else {
obj = *str;
}
int type = I::GetInstanceType(obj) & I::kFullStringRepresentationMask;
*encoding_out = static_cast<Encoding>(type & I::kStringEncodingMask);
if (type == I::kExternalOneByteRepresentationTag ||
type == I::kExternalTwoByteRepresentationTag) {
void* value = I::ReadField<void*>(obj, I::kStringResourceOffset);
resource = static_cast<ExternalStringResourceBase*>(value);
}
return resource;
}
const String::ExternalOneByteStringResource*
String::GetExternalOneByteStringResourceSlow() const {
i::Handle<i::String> str = Utils::OpenHandle(this);
if (str->IsThinString()) {
i::MemoryChunk* chunk = i::MemoryChunk::FromHeapObject(*str);
str = i::Handle<i::String>(i::Handle<i::ThinString>::cast(str)->actual(),
chunk->heap()->isolate());
}
if (i::StringShape(*str).IsExternalOneByte()) {
const void* resource =
i::Handle<i::ExternalOneByteString>::cast(str)->resource();
return reinterpret_cast<const ExternalOneByteStringResource*>(resource);
}
return nullptr;
}
const v8::String::ExternalOneByteStringResource*
v8::String::GetExternalOneByteStringResource() const {
i::Handle<i::String> str = Utils::OpenHandle(this);
if (i::StringShape(*str).IsExternalOneByte()) {
const void* value =
const void* resource =
i::Handle<i::ExternalOneByteString>::cast(str)->resource();
return reinterpret_cast<const ExternalOneByteStringResource*>(value);
return reinterpret_cast<const ExternalOneByteStringResource*>(resource);
} else {
return GetExternalOneByteStringResourceSlow();
return nullptr;
}
}
@ -6937,26 +6860,28 @@ Local<String> v8::String::NewExternal(
bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
i::Handle<i::String> obj = Utils::OpenHandle(this);
i::Isolate* isolate = nullptr;
i::Isolate::FromWritableHeapObject(*obj, &isolate);
DCHECK(isolate != nullptr);
if (obj->IsThinString()) {
obj = i::Handle<i::String>(i::ThinString::cast(*obj)->actual(), isolate);
}
if (!obj->SupportsExternalization()) {
i::Isolate* isolate;
if (!i::Isolate::FromWritableHeapObject(*obj, &isolate)) {
// RO_SPACE strings cannot be externalized.
return false;
}
if (i::StringShape(*obj).IsExternal()) {
return false; // Already an external string.
}
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
if (isolate->heap()->IsInGCPostProcessing()) {
return false;
}
CHECK(resource && resource->data());
bool result = obj->MakeExternal(resource);
DCHECK(result);
DCHECK(obj->IsExternalString());
// Assert that if CanMakeExternal(), then externalizing actually succeeds.
DCHECK(!CanMakeExternal() || result);
if (result) {
DCHECK(obj->IsExternalString());
}
return result;
}
@ -6964,45 +6889,41 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
bool v8::String::MakeExternal(
v8::String::ExternalOneByteStringResource* resource) {
i::Handle<i::String> obj = Utils::OpenHandle(this);
i::Isolate* isolate = nullptr;
i::Isolate::FromWritableHeapObject(*obj, &isolate);
DCHECK(isolate != nullptr);
if (obj->IsThinString()) {
obj = i::Handle<i::String>(i::ThinString::cast(*obj)->actual(), isolate);
}
if (!obj->SupportsExternalization()) {
i::Isolate* isolate;
if (!i::Isolate::FromWritableHeapObject(*obj, &isolate)) {
// RO_SPACE strings cannot be externalized.
return false;
}
if (i::StringShape(*obj).IsExternal()) {
return false; // Already an external string.
}
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
if (isolate->heap()->IsInGCPostProcessing()) {
return false;
}
CHECK(resource && resource->data());
bool result = obj->MakeExternal(resource);
DCHECK(result);
DCHECK(obj->IsExternalString());
// Assert that if CanMakeExternal(), then externalizing actually succeeds.
DCHECK(!CanMakeExternal() || result);
if (result) {
DCHECK(obj->IsExternalString());
}
return result;
}
bool v8::String::CanMakeExternal() {
i::Handle<i::String> obj = Utils::OpenHandle(this);
if (obj->IsExternalString()) return false;
if (obj->IsThinString()) {
i::Isolate* isolate = nullptr;
i::Isolate::FromWritableHeapObject(*obj, &isolate);
DCHECK(isolate != nullptr);
obj = i::Handle<i::String>(i::ThinString::cast(*obj)->actual(), isolate);
return Utils::ToLocal(obj)->CanMakeExternal();
}
if (!obj->SupportsExternalization()) {
i::Isolate* isolate;
if (!i::Isolate::FromWritableHeapObject(*obj, &isolate)) {
// RO_SPACE strings cannot be externalized.
return false;
}
// Only old space strings should be externalized.
return !i::Heap::InNewSpace(*obj);
}

14
src/extensions/externalize-string-extension.cc
View File

@ -84,14 +84,12 @@ void ExternalizeStringExtension::Externalize(
}
}
bool result = false;
v8::String* v8_string = *args[0].As<v8::String>();
Handle<String> string = Utils::OpenHandle(v8_string);
if (!string->SupportsExternalization()) {
Handle<String> string = Utils::OpenHandle(*args[0].As<v8::String>());
if (string->IsExternalString()) {
args.GetIsolate()->ThrowException(
v8::String::NewFromUtf8(args.GetIsolate(),
"string does not support externalization.",
NewStringType::kNormal)
.ToLocalChecked());
"externalizeString() can't externalize twice.",
NewStringType::kNormal).ToLocalChecked());
return;
}
if (string->IsOneByteRepresentation() && !force_two_byte) {
@ -99,14 +97,14 @@ void ExternalizeStringExtension::Externalize(
String::WriteToFlat(*string, data, 0, string->length());
SimpleOneByteStringResource* resource = new SimpleOneByteStringResource(
reinterpret_cast<char*>(data), string->length());
result = v8_string->MakeExternal(resource);
result = string->MakeExternal(resource);
if (!result) delete resource;
} else {
uc16* data = new uc16[string->length()];
String::WriteToFlat(*string, data, 0, string->length());
SimpleTwoByteStringResource* resource = new SimpleTwoByteStringResource(
data, string->length());
result = v8_string->MakeExternal(resource);
result = string->MakeExternal(resource);
if (!result) delete resource;
}
if (!result) {

2
src/heap/heap-inl.h
View File

@ -530,8 +530,6 @@ Isolate* Heap::isolate() {
void Heap::ExternalStringTable::AddString(String* string) {
DCHECK(string->IsExternalString());
DCHECK(!Contains(string));
if (InNewSpace(string)) {
new_space_strings_.push_back(string);
} else {

10
src/heap/heap.cc
View File

@ -2296,16 +2296,6 @@ void Heap::ProtectUnprotectedMemoryChunks() {
unprotected_memory_chunks_.clear();
}
bool Heap::ExternalStringTable::Contains(HeapObject* obj) {
for (size_t i = 0; i < new_space_strings_.size(); ++i) {
if (new_space_strings_[i] == obj) return true;
}
for (size_t i = 0; i < old_space_strings_.size(); ++i) {
if (old_space_strings_[i] == obj) return true;
}
return false;
}
String* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
Object** p) {
MapWord first_word = HeapObject::cast(*p)->map_word();

1
src/heap/heap.h
View File

@ -1481,7 +1481,6 @@ class Heap {
// Registers an external string.
inline void AddString(String* string);
bool Contains(HeapObject* obj);
void IterateAll(RootVisitor* v);
void IterateNewSpaceStrings(RootVisitor* v);

24
src/objects.cc
View File

@ -2573,7 +2573,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
DisallowHeapAllocation no_allocation;
// Externalizing twice leaks the external resource, so it's
// prohibited by the API.
DCHECK(this->SupportsExternalization());
DCHECK(!this->IsExternalString());
DCHECK(!resource->IsCompressible());
#ifdef ENABLE_SLOW_DCHECKS
if (FLAG_enable_slow_asserts) {
@ -2653,7 +2653,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
DisallowHeapAllocation no_allocation;
// Externalizing twice leaks the external resource, so it's
// prohibited by the API.
DCHECK(this->SupportsExternalization());
DCHECK(!this->IsExternalString());
DCHECK(!resource->IsCompressible());
#ifdef ENABLE_SLOW_DCHECKS
if (FLAG_enable_slow_asserts) {
@ -2722,26 +2722,6 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
return true;
}
bool String::SupportsExternalization() {
Isolate* isolate;
if (this->IsThinString()) {
return i::ThinString::cast(this)->actual()->SupportsExternalization();
}
// RO_SPACE strings cannot be externalized.
if (!Isolate::FromWritableHeapObject(this, &isolate)) {
return false;
}
// Already an external string.
if (StringShape(this).IsExternal()) {
return false;
}
return !isolate->heap()->IsInGCPostProcessing();
}
void String::StringShortPrint(StringStream* accumulator, bool show_details) {
int len = length();
if (len > kMaxShortPrintLength) {

1
src/objects/string.h
View File

@ -311,7 +311,6 @@ class String : public Name {
// Externalization.
bool MakeExternal(v8::String::ExternalStringResource* resource);
bool MakeExternal(v8::String::ExternalOneByteStringResource* resource);
bool SupportsExternalization();
// Conversion.
inline bool AsArrayIndex(uint32_t* index);