Commit 9edcb196 authored by Shu-yu Guo's avatar Shu-yu Guo Committed by Commit Bot

Revert "[heap] String::MakeThin can get away without NotifyObjectLayoutChange"

This reverts commit 6e621f84.

Reason for revert: Suspicion of GC stress failures like https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20gc%20stress/30260

Original change's description:
> [heap] String::MakeThin can get away without NotifyObjectLayoutChange
>
> String::MakeThin doesn't need to invoke NotifyObjectLayoutChange because
> ThinString will only introduce tagged values and hence will not
> overwrite recorded slots with untagged values.
>
> Bug: v8:10315
> Change-Id: Iaff9c06cef763462eb57bf3debc5183ae8db6fa0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2448792
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70321}

TBR=ulan@chromium.org,leszeks@chromium.org,dinfuehr@chromium.org

Change-Id: I11c12e25702eb816cf616593d817a6ee3f691188
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10315
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2451029Reviewed-by: 's avatarShu-yu Guo <syg@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70326}
parent 73a8eded
...@@ -3518,19 +3518,13 @@ class SlotCollectingVisitor final : public ObjectVisitor { ...@@ -3518,19 +3518,13 @@ class SlotCollectingVisitor final : public ObjectVisitor {
void Heap::VerifyObjectLayoutChange(HeapObject object, Map new_map) { void Heap::VerifyObjectLayoutChange(HeapObject object, Map new_map) {
if (!FLAG_verify_heap) return; if (!FLAG_verify_heap) return;
// Check that Heap::NotifyObjectLayoutChange was called for object transitions // Check that Heap::NotifyObjectLayout was called for object transitions
// that are not safe for concurrent marking. // that are not safe for concurrent marking.
// If you see this check triggering for a freshly allocated object, // If you see this check triggering for a freshly allocated object,
// use object->set_map_after_allocation() to initialize its map. // use object->set_map_after_allocation() to initialize its map.
if (pending_layout_change_object_.is_null()) { if (pending_layout_change_object_.is_null()) {
if (object.IsJSObject()) { if (object.IsJSObject()) {
DCHECK(!object.map().TransitionRequiresSynchronizationWithGC(new_map)); DCHECK(!object.map().TransitionRequiresSynchronizationWithGC(new_map));
} else if (object.IsString() &&
(new_map == ReadOnlyRoots(this).thin_string_map() ||
new_map == ReadOnlyRoots(this).thin_one_byte_string_map())) {
// When transitioning a string to ThinString,
// Heap::NotifyObjectLayoutChange doesn't need to be invoked because only
// tagged fields are introduced.
} else { } else {
// Check that the set of slots before and after the transition match. // Check that the set of slots before and after the transition match.
SlotCollectingVisitor old_visitor; SlotCollectingVisitor old_visitor;
......
...@@ -117,6 +117,10 @@ void String::MakeThin(Isolate* isolate, String internalized) { ...@@ -117,6 +117,10 @@ void String::MakeThin(Isolate* isolate, String internalized) {
bool has_pointers = StringShape(*this).IsIndirect(); bool has_pointers = StringShape(*this).IsIndirect();
int old_size = this->Size(); int old_size = this->Size();
// Slot invalidation is not necessary here: ThinString only stores tagged
// value, so it can't store an untagged value in a recorded slot.
isolate->heap()->NotifyObjectLayoutChange(*this, no_gc,
InvalidateRecordedSlots::kNo);
bool one_byte = internalized.IsOneByteRepresentation(); bool one_byte = internalized.IsOneByteRepresentation();
Handle<Map> map = one_byte ? isolate->factory()->thin_one_byte_string_map() Handle<Map> map = one_byte ? isolate->factory()->thin_one_byte_string_map()
: isolate->factory()->thin_string_map(); : isolate->factory()->thin_string_map();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment