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

[heap] Fix bug in external strings table cleaning

Browse Source 
After a MinorMC we iterate over the set of young external strings,
finalize the unmarked ones, and clear their slots.
Since MinorMC no longer updates the young string set (to avoid iterating
over the set during evacuation) it may now contain an old string.
If after promoting an external string from young to old,  we get another
MinorMC cycle before we get a full GC (that will "reset" the young and
old string sets), the next MinorMC cycle may treat the promoted string
as unreachable. This is because the string is in old space are is
therefore left unmarked.

Bug: chromium:1412669, v8:12612
Change-Id: I1cacc25b74d9d3dd221c0cc1f0d8b4c4eb83a04d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4219106
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Auto-Submit: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85653}
This commit is contained in:
Omer Katz 2023-02-03 18:33:21 +01:00 committed by V8 LUCI CQ
parent 0253c3a979
commit f18065a3ca
1 changed files with 22 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
src/heap/mark-compact.cc
View File

@ -1354,6 +1354,9 @@ class InternalizedStringTableCleaner final : public RootVisitor {
int pointers_removed_ = 0;
};
enum class ExternalStringTableCleaningMode { kAll, kYoungOnly };
template <ExternalStringTableCleaningMode mode>
class ExternalStringTableCleaner : public RootVisitor {
public:
explicit ExternalStringTableCleaner(Heap* heap) : heap_(heap) {}
@ -1367,19 +1370,22 @@ class ExternalStringTableCleaner : public RootVisitor {
Object the_hole = ReadOnlyRoots(heap_).the_hole_value();
for (FullObjectSlot p = start; p < end; ++p) {
Object o = *p;
if (o.IsHeapObject()) {
HeapObject heap_object = HeapObject::cast(o);
if (marking_state->IsWhite(heap_object)) {
if (o.IsExternalString()) {
heap_->FinalizeExternalString(String::cast(o));
} else {
// The original external string may have been internalized.
DCHECK(o.IsThinString());
}
// Set the entry to the_hole_value (as deleted).
p.store(the_hole);
}
if (!o.IsHeapObject()) continue;
HeapObject heap_object = HeapObject::cast(o);
// MinorMC doesn't update the young strings set and so it may contain
// strings that are already in old space.
if (!marking_state->IsWhite(heap_object)) continue;
if ((mode == ExternalStringTableCleaningMode::kYoungOnly) &&
!Heap::InYoungGeneration(heap_object))
continue;
if (o.IsExternalString()) {
heap_->FinalizeExternalString(String::cast(o));
} else {
// The original external string may have been internalized.
DCHECK(o.IsThinString());
}
// Set the entry to the_hole_value (as deleted).
p.store(the_hole);
}
}
@ -3091,7 +3097,8 @@ void MarkCompactCollector::ClearNonLiveReferences() {
{
TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_CLEAR_EXTERNAL_STRING_TABLE);
ExternalStringTableCleaner external_visitor(heap());
ExternalStringTableCleaner<ExternalStringTableCleaningMode::kAll>
external_visitor(heap());
heap()->external_string_table_.IterateAll(&external_visitor);
heap()->external_string_table_.CleanUpAll();
}
@ -6031,7 +6038,8 @@ void MinorMarkCompactCollector::ClearNonLiveReferences() {
TRACE_GC(heap()->tracer(), GCTracer::Scope::MINOR_MC_CLEAR_STRING_TABLE);
// Internalized strings are always stored in old space, so there is no
// need to clean them here.
ExternalStringTableCleaner external_visitor(heap());
ExternalStringTableCleaner<ExternalStringTableCleaningMode::kYoungOnly>
external_visitor(heap());
heap()->external_string_table_.IterateYoung(&external_visitor);
heap()->external_string_table_.CleanUpYoung();
}