[heap, snapshot] Fix String::MakeThin for background deserialization

During background deserialization strings are inserted into the string
table. When a string was internalized already it needs to be
transitioned into a ThinString using String::MakeThin.

String::MakeThin invokes NotifyObjectSizeChange which will update
the object size cached in the invalidated_slots map. Since this
operation is unsynchronized this is only allowed on the main thread.

However deserialization may also happen on a background thread. In
this case we know that the just allocated object wasn't recorded in
invalidated_slots yet, so UpdateInvalidatedObjectSize can be skipped
for deserialization.

This CL adds an additional argument to String::MakeThin which enables
the caller to skip invoking UpdateInvalidatedObjectSize.

Bug: chromium:1375228
Change-Id: I6291e6844294dfdc5040da9af6486df6d4120888
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3966188
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83797}
This commit is contained in:
Dominik Inführ 2022-10-19 13:57:43 +02:00 committed by V8 LUCI CQ
parent ef0d2f5ca1
commit 48bc3505f0
7 changed files with 70 additions and 21 deletions

View File

@ -3794,24 +3794,46 @@ void Heap::NotifyObjectLayoutChange(
#endif
}
void Heap::NotifyObjectSizeChange(HeapObject object, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots) {
void Heap::NotifyObjectSizeChange(
HeapObject object, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots,
enum UpdateInvalidatedObjectSize update_invalidated_object_size) {
old_size = ALIGN_TO_ALLOCATION_ALIGNMENT(old_size);
new_size = ALIGN_TO_ALLOCATION_ALIGNMENT(new_size);
DCHECK_LE(new_size, old_size);
if (new_size == old_size) return;
UpdateInvalidatedObjectSize(object, new_size);
const bool is_main_thread = LocalHeap::Current() == nullptr;
const bool is_background = LocalHeap::Current() != nullptr;
DCHECK_IMPLIES(is_background,
DCHECK_IMPLIES(!is_main_thread,
clear_recorded_slots == ClearRecordedSlots::kNo);
DCHECK_IMPLIES(!is_main_thread, update_invalidated_object_size ==
UpdateInvalidatedObjectSize::kNo);
const VerifyNoSlotsRecorded verify_no_slots_recorded =
is_background ? VerifyNoSlotsRecorded::kNo : VerifyNoSlotsRecorded::kYes;
if (update_invalidated_object_size == UpdateInvalidatedObjectSize::kYes) {
UpdateInvalidatedObjectSize(object, new_size);
} else {
DCHECK_EQ(update_invalidated_object_size, UpdateInvalidatedObjectSize::kNo);
const ClearFreedMemoryMode clear_memory_mode =
ClearFreedMemoryMode::kDontClearFreedMemory;
#if DEBUG
if (is_main_thread) {
// When running on the main thread we can actually DCHECK that this object
// wasn't recorded in the invalidated_slots map yet.
MemoryChunk* chunk = MemoryChunk::FromHeapObject(object);
DCHECK(!chunk->RegisteredObjectWithInvalidatedSlots<OLD_TO_NEW>(object));
DCHECK(
!chunk->RegisteredObjectWithInvalidatedSlots<OLD_TO_SHARED>(object));
DCHECK_IMPLIES(
incremental_marking()->IsCompacting(),
!chunk->RegisteredObjectWithInvalidatedSlots<OLD_TO_OLD>(object));
}
#endif
}
const auto verify_no_slots_recorded =
is_main_thread ? VerifyNoSlotsRecorded::kYes : VerifyNoSlotsRecorded::kNo;
const auto clear_memory_mode = ClearFreedMemoryMode::kDontClearFreedMemory;
const Address filler = object.address() + new_size;
const int filler_size = old_size - new_size;
@ -3822,6 +3844,11 @@ void Heap::NotifyObjectSizeChange(HeapObject object, int old_size, int new_size,
void Heap::UpdateInvalidatedObjectSize(HeapObject object, int new_size) {
if (!MayContainRecordedSlots(object)) return;
// Updating invalidated_slots is unsychronized and thus needs to happen on the
// main thread.
DCHECK_NULL(LocalHeap::Current());
DCHECK_EQ(isolate()->thread_id(), ThreadId::Current());
if (incremental_marking()->IsCompacting() || gc_state() == MARK_COMPACT) {
MemoryChunk::FromHeapObject(object)
->UpdateInvalidatedObjectSize<OLD_TO_OLD>(object, new_size);

View File

@ -143,6 +143,8 @@ enum ArrayStorageAllocationMode {
enum class ClearRecordedSlots { kYes, kNo };
enum class UpdateInvalidatedObjectSize { kYes, kNo };
enum class InvalidateRecordedSlots { kYes, kNo };
enum class ClearFreedMemoryMode { kClearFreedMemory, kDontClearFreedMemory };
@ -1117,8 +1119,11 @@ class Heap {
// The runtime uses this function to inform the GC of object size changes. The
// GC will fill this area with a filler object and might clear recorded slots
// in that area.
void NotifyObjectSizeChange(HeapObject, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots);
void NotifyObjectSizeChange(
HeapObject, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots,
UpdateInvalidatedObjectSize update_invalidated_object_size =
UpdateInvalidatedObjectSize::kYes);
// ===========================================================================
// Deoptimization support API. ===============================================

View File

@ -438,9 +438,11 @@ void LocalHeap::InvokeGCEpilogueCallbacksInSafepoint(GCType gc_type,
void LocalHeap::NotifyObjectSizeChange(
HeapObject object, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots) {
ClearRecordedSlots clear_recorded_slots,
UpdateInvalidatedObjectSize update_invalidated_object_size) {
heap()->NotifyObjectSizeChange(object, old_size, new_size,
clear_recorded_slots);
clear_recorded_slots,
update_invalidated_object_size);
}
} // namespace internal

View File

@ -154,8 +154,11 @@ class V8_EXPORT_PRIVATE LocalHeap {
AllocationOrigin origin = AllocationOrigin::kRuntime,
AllocationAlignment alignment = kTaggedAligned);
void NotifyObjectSizeChange(HeapObject object, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots);
void NotifyObjectSizeChange(
HeapObject object, int old_size, int new_size,
ClearRecordedSlots clear_recorded_slots,
UpdateInvalidatedObjectSize update_invalidated_object_size =
UpdateInvalidatedObjectSize::kYes);
bool is_main_thread() const { return is_main_thread_; }
bool deserialization_complete() const {

View File

@ -198,7 +198,9 @@ void InitExternalPointerFieldsDuringExternalization(String string, Map new_map,
} // namespace
template <typename IsolateT>
void String::MakeThin(IsolateT* isolate, String internalized) {
void String::MakeThin(
IsolateT* isolate, String internalized,
UpdateInvalidatedObjectSize update_invalidated_object_size) {
DisallowGarbageCollection no_gc;
DCHECK_NE(*this, internalized);
DCHECK(internalized.IsInternalizedString());
@ -258,7 +260,8 @@ void String::MakeThin(IsolateT* isolate, String internalized) {
isolate->heap()->NotifyObjectSizeChange(thin, old_size, ThinString::kSize,
may_contain_recorded_slots
? ClearRecordedSlots::kYes
: ClearRecordedSlots::kNo);
: ClearRecordedSlots::kNo,
update_invalidated_object_size);
} else {
// We don't need special handling for the combination IsLargeObject &&
// may_contain_recorded_slots, because indirect strings never get that
@ -269,9 +272,11 @@ void String::MakeThin(IsolateT* isolate, String internalized) {
}
template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) void String::MakeThin(
Isolate* isolate, String internalized);
Isolate* isolate, String internalized,
UpdateInvalidatedObjectSize update_invalidated_object_size);
template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) void String::MakeThin(
LocalIsolate* isolate, String internalized);
LocalIsolate* isolate, String internalized,
UpdateInvalidatedObjectSize update_invalidated_object_size);
template <typename T>
bool String::MarkForExternalizationDuringGC(Isolate* isolate, T* resource) {

View File

@ -11,6 +11,7 @@
#include "src/base/export-template.h"
#include "src/base/strings.h"
#include "src/common/globals.h"
#include "src/heap/heap.h"
#include "src/objects/instance-type.h"
#include "src/objects/map.h"
#include "src/objects/name.h"
@ -194,7 +195,9 @@ class String : public TorqueGeneratedString<String, Name> {
template <typename IsolateT>
EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
void MakeThin(IsolateT* isolate, String canonical);
void MakeThin(IsolateT* isolate, String canonical,
UpdateInvalidatedObjectSize update_invalidated_object_size =
UpdateInvalidatedObjectSize::kYes);
template <typename Char>
V8_INLINE base::Vector<const Char> GetCharVector(

View File

@ -462,7 +462,11 @@ void Deserializer<IsolateT>::PostProcessNewObject(Handle<Map> map,
String result = *isolate()->string_table()->LookupKey(isolate(), &key);
if (result != raw_obj) {
String::cast(raw_obj).MakeThin(isolate(), result);
// Updating invalidated object size from a background thread would
// race. We are allowed to skip this here since this string hasn't
// transitioned so far.
String::cast(raw_obj).MakeThin(isolate(), result,
UpdateInvalidatedObjectSize::kNo);
// Mutate the given object handle so that the backreference entry is
// also updated.
obj.PatchValue(result);