Commit 45e24fd6 authored by Maya Lekova's avatar Maya Lekova Committed by Commit Bot

Revert "[heap] Do not scan main thread handles when starting marking"

This reverts commit dcd91455.

Reason for revert: Breaks TSAN no-concurrent-marking - https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20no-concurrent-marking/3341/overview

Original change's description:
> [heap] Do not scan main thread handles when starting marking
>
> We do not need to scan main thread handles when starting incremental
> marking. This reduces the time to start incremental marking.
>
> Bug: v8:11645
> Change-Id: Ib99a13e7875f50fbfe5346ac0e186d8960ea1337
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2826124
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#73994}

Bug: v8:11645
Change-Id: I5e2325ef326b79b1807b52384cc5473d126ca6cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2831482
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#73997}
parent ae0752df
......@@ -4493,10 +4493,8 @@ void Heap::IterateRoots(RootVisitor* v, base::EnumSet<SkipRoot> options) {
// Iterate over local handles in handle scopes.
FixStaleLeftTrimmedHandlesVisitor left_trim_visitor(this);
#ifndef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
if (!options.contains(SkipRoot::kMainThreadHandles)) {
isolate_->handle_scope_implementer()->Iterate(&left_trim_visitor);
isolate_->handle_scope_implementer()->Iterate(v);
}
isolate_->handle_scope_implementer()->Iterate(&left_trim_visitor);
isolate_->handle_scope_implementer()->Iterate(v);
#endif
safepoint_->Iterate(&left_trim_visitor);
......
......@@ -176,7 +176,6 @@ enum class SkipRoot {
kGlobalHandles,
kOldGeneration,
kStack,
kMainThreadHandles,
kUnserializable,
kWeak
};
......
......@@ -318,9 +318,7 @@ void IncrementalMarking::MarkRoots() {
IncrementalMarkingRootMarkingVisitor visitor(this);
heap_->IterateRoots(
&visitor,
base::EnumSet<SkipRoot>{SkipRoot::kStack, SkipRoot::kMainThreadHandles,
SkipRoot::kWeak});
&visitor, base::EnumSet<SkipRoot>{SkipRoot::kStack, SkipRoot::kWeak});
}
bool IncrementalMarking::ShouldRetainMap(Map map, int age) {
......
......@@ -269,10 +269,6 @@ TEST(ArrayBuffer_LivePromotion) {
Handle<JSArrayBuffer> buf = v8::Utils::OpenHandle(*ab);
root->set(0, *buf); // Buffer that should be promoted as live.
}
// Store array in Global such that it is part of the root set when
// starting incremental marking.
v8::Global<Value> global_root(CcTest::isolate(),
Utils::ToLocal(Handle<Object>::cast(root)));
heap::SimulateIncrementalMarking(heap, true);
CHECK(IsTracked(heap, JSArrayBuffer::cast(root->get(0))));
heap::GcAndSweep(heap, NEW_SPACE);
......
......@@ -112,12 +112,6 @@ TEST(ConcurrentMarkingMarkedBytes) {
Handle<FixedArray> root = isolate->factory()->NewFixedArray(1000000);
CcTest::CollectAllGarbage();
if (!heap->incremental_marking()->IsStopped()) return;
// Store array in Global such that it is part of the root set when
// starting incremental marking.
v8::Global<Value> global_root(CcTest::isolate(),
Utils::ToLocal(Handle<Object>::cast(root)));
heap::SimulateIncrementalMarking(heap, false);
heap->concurrent_marking()->Join();
CHECK_GE(heap->concurrent_marking()->TotalMarkedBytes(), root->Size());
......
......@@ -2304,11 +2304,6 @@ TEST(InstanceOfStubWriteBarrier) {
v8::HandleScope outer_scope(CcTest::isolate());
v8::Local<v8::Context> ctx = CcTest::isolate()->GetCurrentContext();
// Store native context in global as well to make it part of the root set when
// starting incremental marking. This will ensure that function will be part
// of the transitive closure during incremental marking.
v8::Global<v8::Context> global_ctx(CcTest::isolate(), ctx);
{
v8::HandleScope scope(CcTest::isolate());
CompileRun(
......@@ -5223,10 +5218,6 @@ void CheckMapRetainingFor(int n) {
Handle<Context> context = Utils::OpenHandle(*ctx);
CHECK(context->IsNativeContext());
Handle<NativeContext> native_context = Handle<NativeContext>::cast(context);
// This global is used to visit the object's constructor alive when starting
// incremental marking. The native context keeps the constructor alive. The
// constructor needs to be alive to retain the map.
v8::Global<v8::Context> global_ctxt(CcTest::isolate(), ctx);
ctx->Enter();
Handle<WeakFixedArray> array_with_map =
......@@ -5711,17 +5702,11 @@ TEST(Regress598319) {
get().set(i, *tmp);
}
}
global_root.Reset(CcTest::isolate(),
Utils::ToLocal(Handle<Object>::cast(root)));
}
FixedArray get() { return FixedArray::cast(root->get(0)); }
Handle<FixedArray> root;
// Store array in global as well to make it part of the root set when
// starting incremental marking.
v8::Global<Value> global_root;
} arr(isolate, kNumberOfObjects);
CHECK_EQ(arr.get().length(), kNumberOfObjects);
......
......@@ -75,11 +75,7 @@ HEAP_TEST(WriteBarrier_MarkingExtension) {
CHECK(collector->marking_state()->IsWhite(host));
CHECK(!extension->IsMarked());
WriteBarrier::Marking(host, extension);
// Concurrent marking barrier should mark this object.
CHECK_EQ(V8_CONCURRENT_MARKING_BOOL, extension->IsMarked());
// Keep object alive using the global handle.
v8::Global<ArrayBuffer> global_host(CcTest::isolate(),
Utils::ToLocal(handle(host, isolate)));
heap::SimulateIncrementalMarking(CcTest::heap(), true);
CHECK(collector->marking_state()->IsBlack(host));
CHECK(extension->IsMarked());
......
......@@ -892,11 +892,6 @@ TEST(JSWeakRefScavengedInWorklist) {
weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
}
// Store weak_ref in Global such that it is part of the root set when
// starting incremental marking.
v8::Global<Value> global_weak_ref(
CcTest::isolate(), Utils::ToLocal(Handle<Object>::cast(weak_ref)));
// Do marking. This puts the WeakRef above into the js_weak_refs worklist
// since its target isn't marked.
CHECK(
......@@ -939,10 +934,6 @@ TEST(JSWeakRefTenuredInWorklist) {
weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
}
// Store weak_ref such that it is part of the root set when starting
// incremental marking.
v8::Global<Value> global_weak_ref(
CcTest::isolate(), Utils::ToLocal(Handle<Object>::cast(weak_ref)));
JSWeakRef old_weak_ref_location = *weak_ref;
// Do marking. This puts the WeakRef above into the js_weak_refs worklist
......
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