Skip to content

Commit

Permalink
Oilpan: Use InAtomicMarkingPause to remove GCForbidenScope and NoAllo…
Browse files Browse the repository at this point in the history
…cationScope usage

Use InAtomicMarkingPause to remove GCForbidenScope and NoAllocationScope usage

Change-Id: If723098063ea02d80dee0ae38257cdfa40959d66
Reviewed-on: https://chromium-review.googlesource.com/978792
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546002}
  • Loading branch information
Keishi Hattori authored and Commit Bot committed Mar 27, 2018
1 parent aeecb4f commit 4af3b77
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 15 deletions.
2 changes: 0 additions & 2 deletions third_party/WebKit/Source/platform/heap/HeapTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4007,7 +4007,6 @@ TEST(HeapTest, CheckAndMarkPointer) {
// to allocate anything again. We do this by forcing a GC after doing the
// checkAndMarkPointer tests.
{
ThreadState::GCForbiddenScope gc_scope(ThreadState::Current());
TestGCScope scope(BlinkGC::kHeapPointersOnStack);
MarkingVisitor visitor(ThreadState::Current(),
MarkingVisitor::kGlobalMarking);
Expand All @@ -4032,7 +4031,6 @@ TEST(HeapTest, CheckAndMarkPointer) {
// however we don't rely on that below since we don't have any allocations.
ClearOutOldGarbage();
{
ThreadState::GCForbiddenScope gc_scope(ThreadState::Current());
TestGCScope scope(BlinkGC::kHeapPointersOnStack);
MarkingVisitor visitor(ThreadState::Current(),
MarkingVisitor::kGlobalMarking);
Expand Down
4 changes: 1 addition & 3 deletions third_party/WebKit/Source/platform/heap/MarkingVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ MarkingVisitor::MarkingVisitor(ThreadState* state, MarkingMode marking_mode)
weak_callback_worklist_(Heap().GetWeakCallbackWorklist(),
WorklistTaskId::MainThread),
marking_mode_(marking_mode) {
// See ThreadState::runScheduledGC() why we need to already be in a
// GCForbiddenScope before any safe point is entered.
DCHECK(state->IsGCForbidden());
DCHECK(state->InAtomicMarkingPause());
#if DCHECK_IS_ON()
DCHECK(state->CheckThread());
#endif // DCHECK_IS_ON
Expand Down
8 changes: 0 additions & 8 deletions third_party/WebKit/Source/platform/heap/ThreadState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1302,8 +1302,6 @@ void ThreadState::CollectGarbage(BlinkGC::StackState stack_state,
RUNTIME_CALL_TIMER_SCOPE_IF_ISOLATE_EXISTS(
GetIsolate(), RuntimeCallStats::CounterId::kCollectGarbage);

GCForbiddenScope gc_forbidden_scope(this);

{
AtomicPauseScope atomic_pause_scope(this);
{
Expand Down Expand Up @@ -1403,9 +1401,6 @@ void ThreadState::MarkPhasePrologue(BlinkGC::StackState stack_state,
void ThreadState::MarkPhaseVisitRoots() {
double start_time = WTF::CurrentTimeTicksInMilliseconds();

// Disallow allocation during garbage collection (but not during the
// finalization that happens when the visitorScope is torn down).
NoAllocationScope no_allocation_scope(this);
// StackFrameDepth should be disabled so we don't trace most of the object
// graph in one incremental marking step.
DCHECK(!Heap().GetStackFrameDepth().IsEnabled());
Expand All @@ -1428,9 +1423,6 @@ void ThreadState::MarkPhaseVisitRoots() {
bool ThreadState::MarkPhaseAdvanceMarking(double deadline_seconds) {
double start_time = WTF::CurrentTimeTicksInMilliseconds();

// Disallow allocation during garbage collection (but not during the
// finalization that happens when the visitorScope is torn down).
NoAllocationScope no_allocation_scope(this);
StackFrameDepthScope stack_depth_scope(&Heap().GetStackFrameDepth());

// 3. Transitive closure to trace objects including ephemerons.
Expand Down
9 changes: 7 additions & 2 deletions third_party/WebKit/Source/platform/heap/ThreadState.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,11 @@ class PLATFORM_EXPORT ThreadState {

// Support for disallowing allocation. Mainly used for sanity
// checks asserts.
bool IsAllocationAllowed() const { return !no_allocation_count_; }
bool IsAllocationAllowed() const {
// Allocation is not allowed during atomic marking pause, but it is allowed
// during atomic sweeping pause.
return !InAtomicMarkingPause() && !no_allocation_count_;
}
void EnterNoAllocationScope() { no_allocation_count_++; }
void LeaveNoAllocationScope() { no_allocation_count_--; }
bool IsWrapperTracingForbidden() { return IsMixinInConstruction(); }
Expand Down Expand Up @@ -406,14 +410,15 @@ class PLATFORM_EXPORT ThreadState {
class AtomicPauseScope final {
public:
explicit AtomicPauseScope(ThreadState* thread_state)
: thread_state_(thread_state) {
: thread_state_(thread_state), gc_forbidden_scope(thread_state) {
thread_state_->EnterAtomicPause();
}
~AtomicPauseScope() { thread_state_->LeaveAtomicPause(); }

private:
ThreadState* const thread_state_;
ScriptForbiddenScope script_forbidden_scope;
GCForbiddenScope gc_forbidden_scope;
};

void FlushHeapDoesNotContainCacheIfNeeded();
Expand Down

0 comments on commit 4af3b77

Please sign in to comment.