Oilpan: Rename weak callback related methods (issue 1149943003 by haraken@chromium.org)

[har...@chromium.org]

Reviewers: oilpan-reviews, keishi,

Message:
PTAL

A simple CL.


Description:
Oilpan: Rename weak callback related methods

Currently methods for global weak processings and methods for thread-local
weak processings have confusing names. For example,
Heap::pushWeakPointerCallback
pushes weak callbacks for thread-local weak processings whereas
Heap::popAndInvokeWeakPointerCallback pops weak callbacks for global weak
processings.

This CL renames those things to make what is global and what is thread-local
clearer.

BUG=475801

Please review this at https://codereview.chromium.org/1149943003/

Base URL: svn://svn.chromium.org/blink/trunk

Affected files (+30, -30 lines):
M Source/platform/heap/Heap.h
M Source/platform/heap/Heap.cpp
M Source/platform/heap/MarkingVisitorImpl.h
M Source/platform/heap/ThreadState.h
M Source/platform/heap/ThreadState.cpp


Index: Source/platform/heap/Heap.cpp
diff --git a/Source/platform/heap/Heap.cpp b/Source/platform/heap/Heap.cpp
index
c938b3a3ffe05e998fc64b37b3048150329650cc..3337c6645899f48dab48a422e4fc58d21cffc513
100644
--- a/Source/platform/heap/Heap.cpp
+++ b/Source/platform/heap/Heap.cpp
@@ -1613,7 +1613,7 @@ void Heap::init()
ThreadState::init();
s_markingStack = new CallbackStack();
s_postMarkingCallbackStack = new CallbackStack();
- s_weakCallbackStack = new CallbackStack();
+ s_globalWeakCallbackStack = new CallbackStack();
s_ephemeronStack = new CallbackStack();
s_heapDoesNotContainCache = new HeapDoesNotContainCache();
s_markingVisitor = new MarkingVisitor<Visitor::GlobalMarking>();
@@ -1648,8 +1648,8 @@ void Heap::doShutdown()
s_freePagePool = nullptr;
delete s_orphanedPagePool;
s_orphanedPagePool = nullptr;
- delete s_weakCallbackStack;
- s_weakCallbackStack = nullptr;
+ delete s_globalWeakCallbackStack;
+ s_globalWeakCallbackStack = nullptr;
delete s_postMarkingCallbackStack;
s_postMarkingCallbackStack = nullptr;
delete s_markingStack;
@@ -1794,29 +1794,29 @@ bool Heap::popAndInvokePostMarkingCallback(Visitor*
visitor)
return false;
}

-void Heap::pushWeakCellPointerCallback(void** cell, WeakPointerCallback
callback)
+void Heap::pushGlobalWeakCallback(void** cell, WeakPointerCallback
callback)
{
ASSERT(!Heap::orphanedPagePool()->contains(cell));
- CallbackStack::Item* slot = s_weakCallbackStack->allocateEntry();
+ CallbackStack::Item* slot = s_globalWeakCallbackStack->allocateEntry();
*slot = CallbackStack::Item(cell, callback);
}

-void Heap::pushWeakPointerCallback(void* closure, void* object,
WeakPointerCallback callback)
+void Heap::pushThreadLocalWeakCallback(void* closure, void* object,
WeakPointerCallback callback)
{
BasePage* page = pageFromObject(object);
ASSERT(!page->orphaned());
ThreadState* state = page->heap()->threadState();
- state->pushWeakPointerCallback(closure, callback);
+ state->pushThreadLocalWeakCallback(closure, callback);
}

-bool Heap::popAndInvokeWeakPointerCallback(Visitor* visitor)
+bool Heap::popAndInvokeGlobalWeakCallback(Visitor* visitor)
{
// For weak processing we should never reach orphaned pages since
orphaned
// pages are not traced and thus objects on those pages are never be
// registered as objects on orphaned pages. We cannot assert this here
// since we might have an off-heap collection. We assert it in
// Heap::pushWeakPointerCallback.
- if (CallbackStack::Item* item = s_weakCallbackStack->pop()) {
+ if (CallbackStack::Item* item = s_globalWeakCallbackStack->pop()) {
item->call(visitor);
return true;
}
@@ -2040,7 +2040,7 @@ void Heap::globalWeakProcessing(Visitor*
markingVisitor)
{
TRACE_EVENT0("blink_gc", "Heap::globalWeakProcessing");
// Call weak callbacks on objects that may now be pointing to dead
objects.
- while (popAndInvokeWeakPointerCallback(markingVisitor)) { }
+ while (popAndInvokeGlobalWeakCallback(markingVisitor)) { }

// It is not permitted to trace pointers of live objects in the weak
// callback phase, so the marking stack should still be empty here.
@@ -2216,7 +2216,7 @@ void Heap::resetHeapCounters()
Visitor* Heap::s_markingVisitor;
CallbackStack* Heap::s_markingStack;
CallbackStack* Heap::s_postMarkingCallbackStack;
-CallbackStack* Heap::s_weakCallbackStack;
+CallbackStack* Heap::s_globalWeakCallbackStack;
CallbackStack* Heap::s_ephemeronStack;
HeapDoesNotContainCache* Heap::s_heapDoesNotContainCache;
bool Heap::s_shutdownCalled = false;
Index: Source/platform/heap/Heap.h
diff --git a/Source/platform/heap/Heap.h b/Source/platform/heap/Heap.h
index
533fc3ebb40222234b360bbae9db214d689fadbf..5073e373974a91edf5c80fc6211775a2b5d27a31
100644
--- a/Source/platform/heap/Heap.h
+++ b/Source/platform/heap/Heap.h
@@ -859,13 +859,13 @@ public:
// the containerObject can be the same thing, but the containerObject
is
// constrained to be on the heap, since the heap is used to identify
the
// correct thread.
- static void pushWeakPointerCallback(void* closure, void*
containerObject, WeakPointerCallback);
+ static void pushThreadLocalWeakCallback(void* closure, void*
containerObject, WeakPointerCallback);

- // Similar to the more general pushWeakPointerCallback, but cell
+ // Similar to the more general pushThreadLocalWeakCallback, but cell
// pointer callbacks are added to a static callback work list and the
weak
// callback is performed on the thread performing garbage collection.
This
// is OK because cells are just cleared and no deallocation can happen.
- static void pushWeakCellPointerCallback(void** cell,
WeakPointerCallback);
+ static void pushGlobalWeakCallback(void** cell, WeakPointerCallback);

// Pop the top of a marking stack and call the callback with the
visitor
// and the object. Returns false when there is nothing more to do.
@@ -879,7 +879,7 @@ public:
// Remove an item from the weak callback work list and call the
callback
// with the visitor and the closure pointer. Returns false when there
is
// nothing more to do.
- static bool popAndInvokeWeakPointerCallback(Visitor*);
+ static bool popAndInvokeGlobalWeakCallback(Visitor*);

// Register an ephemeron table for fixed-point iteration.
static void registerWeakTable(void* containerObject,
EphemeronCallback, EphemeronCallback);
@@ -1014,7 +1014,7 @@ private:
static Visitor* s_markingVisitor;
static CallbackStack* s_markingStack;
static CallbackStack* s_postMarkingCallbackStack;
- static CallbackStack* s_weakCallbackStack;
+ static CallbackStack* s_globalWeakCallbackStack;
static CallbackStack* s_ephemeronStack;
static HeapDoesNotContainCache* s_heapDoesNotContainCache;
static bool s_shutdownCalled;
Index: Source/platform/heap/MarkingVisitorImpl.h
diff --git a/Source/platform/heap/MarkingVisitorImpl.h
b/Source/platform/heap/MarkingVisitorImpl.h
index
c7da92c43696c31ebdccef7f7ef387552618d043..017d98526e078694b9a1a53fd4a68185dc782697
100644
--- a/Source/platform/heap/MarkingVisitorImpl.h
+++ b/Source/platform/heap/MarkingVisitorImpl.h
@@ -69,7 +69,7 @@ protected:

inline void registerWeakMembers(const void* closure, const void*
objectPointer, WeakPointerCallback callback)
{
- Heap::pushWeakPointerCallback(const_cast<void*>(closure),
const_cast<void*>(objectPointer), callback);
+ Heap::pushThreadLocalWeakCallback(const_cast<void*>(closure),
const_cast<void*>(objectPointer), callback);
}

inline void registerWeakTable(const void* closure, EphemeronCallback
iterationCallback, EphemeronCallback iterationDoneCallback)
@@ -119,7 +119,7 @@ protected:
protected:
inline void registerWeakCellWithCallback(void** cell,
WeakPointerCallback callback)
{
- Heap::pushWeakCellPointerCallback(cell, callback);
+ Heap::pushGlobalWeakCallback(cell, callback);
}

private:
Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp
b/Source/platform/heap/ThreadState.cpp
index
a1ea13a398166e3a82727aaa15339810fc7d4dc3..8020f0fecd954fee32fa34c65933aaaa3d5ed214
100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -126,14 +126,14 @@ ThreadState::ThreadState()
m_likelyToBePromptlyFreed = adoptArrayPtr(new
int[likelyToBePromptlyFreedArraySize]);
clearHeapAges();

- m_weakCallbackStack = new CallbackStack();
+ m_threadLocalWeakCallbackStack = new CallbackStack();
}

ThreadState::~ThreadState()
{
checkThread();
- delete m_weakCallbackStack;
- m_weakCallbackStack = nullptr;
+ delete m_threadLocalWeakCallbackStack;
+ m_threadLocalWeakCallbackStack = nullptr;
for (int i = 0; i < NumberOfHeaps; ++i)
delete m_heaps[i];
deleteAllValues(m_interruptors);
@@ -469,20 +469,20 @@ void ThreadState::incrementMarkedObjectsAge()
}
#endif

-void ThreadState::pushWeakPointerCallback(void* object,
WeakPointerCallback callback)
+void ThreadState::pushThreadLocalWeakCallback(void* object,
WeakPointerCallback callback)
{
- CallbackStack::Item* slot = m_weakCallbackStack->allocateEntry();
+ CallbackStack::Item* slot =
m_threadLocalWeakCallbackStack->allocateEntry();
*slot = CallbackStack::Item(object, callback);
}

-bool ThreadState::popAndInvokeWeakPointerCallback(Visitor* visitor)
+bool ThreadState::popAndInvokeThreadLocalWeakCallback(Visitor* visitor)
{
// For weak processing we should never reach orphaned pages since
orphaned
// pages are not traced and thus objects on those pages are never be
// registered as objects on orphaned pages. We cannot assert this here
since
// we might have an off-heap collection. We assert it in
- // Heap::pushWeakPointerCallback.
- if (CallbackStack::Item* item = m_weakCallbackStack->pop()) {
+ // Heap::pushThreadLocalWeakCallback.
+ if (CallbackStack::Item* item = m_threadLocalWeakCallbackStack->pop())
{
item->call(visitor);
return true;
}
@@ -882,7 +882,7 @@ void ThreadState::preSweep()
{

TRACE_EVENT0("blink_gc", "ThreadState::threadLocalWeakProcessing");
// Perform thread-specific weak processing.
- while
(popAndInvokeWeakPointerCallback(Heap::s_markingVisitor)) { }
+ while
(popAndInvokeThreadLocalWeakCallback(Heap::s_markingVisitor)) { }
}
{

TRACE_EVENT0("blink_gc", "ThreadState::invokePreFinalizers");
Index: Source/platform/heap/ThreadState.h
diff --git a/Source/platform/heap/ThreadState.h
b/Source/platform/heap/ThreadState.h
index
6c54dfed56576c33f5dcf76cf1241383f646699a..68b396c1e8419423050ae932eeb36d4e37be0f7e
100644
--- a/Source/platform/heap/ThreadState.h
+++ b/Source/platform/heap/ThreadState.h
@@ -498,8 +498,8 @@ public:
void reportMarkSweepStats(const char* statsName, const
ClassAgeCountsMap&) const;
#endif

- void pushWeakPointerCallback(void*, WeakPointerCallback);
- bool popAndInvokeWeakPointerCallback(Visitor*);
+ void pushThreadLocalWeakCallback(void*, WeakPointerCallback);
+ bool popAndInvokeThreadLocalWeakCallback(Visitor*);

size_t objectPayloadSizeForTesting();
void prepareHeapForTermination();
@@ -697,7 +697,7 @@ private:
bool m_shouldFlushHeapDoesNotContainCache;
GCState m_gcState;

- CallbackStack* m_weakCallbackStack;
+ CallbackStack* m_threadLocalWeakCallbackStack;
HashMap<void*, bool (*)(void*, Visitor&)> m_preFinalizers;

v8::Isolate* m_isolate;

[sigb...@opera.com]

https://codereview.chromium.org/1149943003/diff/1/Source/platform/heap/Heap.h
File Source/platform/heap/Heap.h (right):

https://codereview.chromium.org/1149943003/diff/1/Source/platform/heap/Heap.h#newcode862
Source/platform/heap/Heap.h:862: static void

pushThreadLocalWeakCallback(void* closure, void* containerObject,
WeakPointerCallback);

Wouldn't it make sense to rename WeakPointerCallback at the same time?

https://codereview.chromium.org/1149943003/

[har...@chromium.org]

https://codereview.chromium.org/1149943003/diff/1/Source/platform/heap/Heap.h
File Source/platform/heap/Heap.h (right):

https://codereview.chromium.org/1149943003/diff/1/Source/platform/heap/Heap.h#newcode862
Source/platform/heap/Heap.h:862: static void
pushThreadLocalWeakCallback(void* closure, void* containerObject,
WeakPointerCallback);

On 2015/05/22 09:07:48, sof wrote:
> Wouldn't it make sense to rename WeakPointerCallback at the same time?

Done.

https://codereview.chromium.org/1149943003/

[sigb...@opera.com]

lgtm



https://codereview.chromium.org/1149943003/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1149943003/20001


https://codereview.chromium.org/1149943003/

[commi...@chromium.org]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195779

https://codereview.chromium.org/1149943003/