Reviewers: jochen,
https://codereview.chromium.org/1209403005/diff/1/src/global-handles.cc
File src/global-handles.cc (right):
https://codereview.chromium.org/1209403005/diff/1/src/global-handles.cc#newcode525
src/global-handles.cc:525: };
On 2015/07/03 at 12:42:27, jochen wrote:
nit. DISALLOW_COPY_AND_ASSIGN(PendingPhantomCallbacksSecondPassTask)
Done.
Description:
Let the second pass phantom callbacks run in a separate task on the
foreground
thread.
[email protected]
LOG=y
BUG=
Please review this at https://codereview.chromium.org/1209403005/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+61, -8 lines):
M src/global-handles.h
M src/global-handles.cc
M src/list.h
M src/list-inl.h
M test/cctest/cctest.h
M test/cctest/cctest.cc
M test/cctest/test-api.cc
Index: src/global-handles.cc
diff --git a/src/global-handles.cc b/src/global-handles.cc
index
aa6542baee922f8f4e517cd6764f8b8ed71290ee..af1aa7e48e3b478cd2db3b520ffd65fb394c50dd
100644
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -495,6 +495,37 @@ class GlobalHandles::NodeIterator {
DISALLOW_COPY_AND_ASSIGN(NodeIterator);
};
+class GlobalHandles::PendingPhantomCallbacksSecondPassTask : public
v8::Task {
+ public:
+ // Takes ownership of the contents of pending_phantom_callbacks, leaving
it in
+ // the same state it would be after a call to Clear().
+ PendingPhantomCallbacksSecondPassTask(
+ List<PendingPhantomCallback>* pending_phantom_callbacks, Isolate*
isolate)
+ : isolate_(isolate) {
+ pending_phantom_callbacks_.Swap(pending_phantom_callbacks);
+ }
+
+ ~PendingPhantomCallbacksSecondPassTask() override {}
+
+ void Run() override {
+ while (pending_phantom_callbacks_.length() != 0) {
+ auto callback = pending_phantom_callbacks_.RemoveLast();
+ DCHECK(callback.node() == nullptr);
+ // No second pass callback required.
+ if (callback.callback() == nullptr) continue;
+ // Fire second pass callback
+ callback.Invoke(isolate_);
+ }
+ pending_phantom_callbacks_.Clear();
+ }
+
+ private:
+ List<PendingPhantomCallback> pending_phantom_callbacks_;
+ Isolate* isolate_;
+
+ DISALLOW_COPY_AND_ASSIGN(PendingPhantomCallbacksSecondPassTask);
+};
+
GlobalHandles::GlobalHandles(Isolate* isolate)
: isolate_(isolate),
@@ -804,14 +835,11 @@ int GlobalHandles::DispatchPendingPhantomCallbacks() {
freed_nodes++;
}
}
- // The second pass empties the list.
- while (pending_phantom_callbacks_.length() != 0) {
- auto callback = pending_phantom_callbacks_.RemoveLast();
- DCHECK(callback.node() == nullptr);
- // No second pass callback required.
- if (callback.callback() == nullptr) continue;
- // Fire second pass callback.
- callback.Invoke(isolate());
+ if (pending_phantom_callbacks_.length() > 0) {
+ auto* task = new PendingPhantomCallbacksSecondPassTask(
+ &pending_phantom_callbacks_, isolate());
+ V8::GetCurrentPlatform()->CallOnForegroundThread(
+ reinterpret_cast<v8::Isolate*>(isolate()), task);
}
pending_phantom_callbacks_.Clear();
return freed_nodes;
Index: src/global-handles.h
diff --git a/src/global-handles.h b/src/global-handles.h
index
6724847303d4188ffdab4fb31c859a9b85d2a410..a620e2366441dfc2356dd5136c1007909a44e23d
100644
--- a/src/global-handles.h
+++ b/src/global-handles.h
@@ -298,6 +298,7 @@ class GlobalHandles {
class NodeBlock;
class NodeIterator;
class PendingPhantomCallback;
+ class PendingPhantomCallbacksSecondPassTask;
Isolate* isolate_;
Index: src/list-inl.h
diff --git a/src/list-inl.h b/src/list-inl.h
index
98f0343fa57f0b84aff9254bf6c1bed7b482e56a..47653ef145bdc0ae2b53a2a2004d4c96506672ee
100644
--- a/src/list-inl.h
+++ b/src/list-inl.h
@@ -125,6 +125,12 @@ bool List<T, P>::RemoveElement(const T& elm) {
return false;
}
+template <typename T, class P>
+void List<T, P>::Swap(List<T, P>* list) {
+ std::swap(data_, list->data_);
+ std::swap(length_, list->length_);
+ std::swap(capacity_, list->capacity_);
+}
template<typename T, class P>
void List<T, P>::Allocate(int length, P allocator) {
Index: src/list.h
diff --git a/src/list.h b/src/list.h
index
b636449c423b9d9ea7a184de78a3580a49c5780e..8021a9fbed1e5d955cbf910107f1c26d948ab5f6
100644
--- a/src/list.h
+++ b/src/list.h
@@ -5,6 +5,8 @@
#ifndef V8_LIST_H_
#define V8_LIST_H_
+#include <algorithm>
+
#include "src/checks.h"
#include "src/utils.h"
@@ -137,6 +139,9 @@ class List {
// Drop the last 'count' elements from the list.
INLINE(void RewindBy(int count)) { Rewind(length_ - count); }
+ // Swaps the contents of the two lists.
+ INLINE(void Swap(List<T, AllocationPolicy>* list));
+
// Halve the capacity if fill level is less than a quarter.
INLINE(void Trim(AllocationPolicy allocator = AllocationPolicy()));
Index: test/cctest/cctest.cc
diff --git a/test/cctest/cctest.cc b/test/cctest/cctest.cc
index
b5771ff6550681526cda8cd6a9a3c36411645482..e722d37153134c18fa7d81dcd7155e30429db12e
100644
--- a/test/cctest/cctest.cc
+++ b/test/cctest/cctest.cc
@@ -97,6 +97,7 @@ void CcTest::Run() {
}
callback_();
if (initialize_) {
+ EmptyMessageLoop(isolate_);
isolate_->Exit();
}
}
Index: test/cctest/cctest.h
diff --git a/test/cctest/cctest.h b/test/cctest/cctest.h
index
cc9edc801f5e268b17476178cea243b51cb2495b..d8288f8d1942c50f1e1ff02b02c1848b6940bab3
100644
--- a/test/cctest/cctest.h
+++ b/test/cctest/cctest.h
@@ -28,6 +28,7 @@
#ifndef CCTEST_H_
#define CCTEST_H_
+#include "include/libplatform/libplatform.h"
#include "src/v8.h"
#ifndef TEST
@@ -594,6 +595,13 @@ static inline void EnableDebugger() {
static inline void DisableDebugger() {
v8::Debug::SetDebugEventListener(NULL); }
+static inline void EmptyMessageLoop(v8::Isolate* isolate) {
+ while
(v8::platform::PumpMessageLoop(v8::internal::V8::GetCurrentPlatform(),
+ isolate))
+ ;
+}
+
+
// Helper class for new allocations tracking and checking.
// To use checking of JS allocations tracking in a test,
// just create an instance of this class.
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
1433b7bed6b86a6a1a664818e34bb47c786618de..a5131787e3dd324cde56f831105b170d03115196
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -3275,6 +3275,7 @@ TEST(TwoPassPhantomCallbacks) {
}
CHECK_EQ(static_cast<int>(kLength), instance_counter);
CcTest::heap()->CollectAllGarbage();
+ EmptyMessageLoop(isolate);
CHECK_EQ(0, instance_counter);
}
@@ -3293,6 +3294,7 @@ TEST(TwoPassPhantomCallbacksNestedGc) {
array[15]->MarkTriggerGc();
CHECK_EQ(static_cast<int>(kLength), instance_counter);
CcTest::heap()->CollectAllGarbage();
+ EmptyMessageLoop(isolate);
CHECK_EQ(0, instance_counter);
}
@@ -6806,6 +6808,7 @@ THREADED_TEST(GCFromWeakCallbacks) {
v8::WeakCallbackType::kParameter);
object.handle.MarkIndependent();
invoke_gc[outer_gc]();
+ EmptyMessageLoop(isolate);
CHECK(object.flag);
}
}
@@ -11818,6 +11821,7 @@
THREADED_TEST(NoGlobalHandlesOrphaningDueToWeakCallback) {
handle3.SetWeak(&handle3, HandleCreatingCallback1,
v8::WeakCallbackType::kParameter);
CcTest::heap()->CollectAllGarbage();
+ EmptyMessageLoop(isolate);
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
