Skip to content

Commit e6e5339

Browse files
epertosoCommit bot
epertoso
authored and
Commit bot
committed
Let the second pass phantom callbacks run in a separate task on the foreground thread.
R=jochen@chromium.org LOG=y BUG= Review URL: https://codereview.chromium.org/1209403005 Cr-Commit-Position: refs/heads/master@{#29680}
1 parent 9386b86 commit e6e5339

14 files changed

Lines changed: 128 additions & 30 deletions

File tree

‎include/v8-util.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ class DefaultGlobalMapTraits : public StdMapTraits<K, V> {
133133
return K();
134134
}
135135
static void DisposeCallbackData(WeakCallbackDataType* data) {}
136+
static void OnWeakCallback(
137+
const WeakCallbackInfo<WeakCallbackDataType>& data) {}
136138
static void Dispose(Isolate* isolate, Global<V> value, K key) {}
137139
// This is a second pass callback, so SetSecondPassCallback cannot be called.
138140
static void DisposeWeak(const WeakCallbackInfo<WeakCallbackDataType>& data) {}
@@ -452,7 +454,7 @@ class GlobalValueMap : public PersistentValueMapBase<K, V, Traits> {
452454
: WeakCallbackType::kParameter;
453455
Local<V> value(Local<V>::New(this->isolate(), *persistent));
454456
persistent->template SetWeak<typename Traits::WeakCallbackDataType>(
455-
Traits::WeakCallbackParameter(this, key, value), FirstWeakCallback,
457+
Traits::WeakCallbackParameter(this, key, value), OnWeakCallback,
456458
callback_type);
457459
}
458460
PersistentContainerValue old_value =
@@ -471,12 +473,13 @@ class GlobalValueMap : public PersistentValueMapBase<K, V, Traits> {
471473
}
472474

473475
private:
474-
static void FirstWeakCallback(
476+
static void OnWeakCallback(
475477
const WeakCallbackInfo<typename Traits::WeakCallbackDataType>& data) {
476478
if (Traits::kCallbackType != kNotWeak) {
477479
auto map = Traits::MapFromWeakCallbackInfo(data);
478480
K key = Traits::KeyFromWeakCallbackInfo(data);
479481
map->RemoveWeak(key);
482+
Traits::OnWeakCallback(data);
480483
data.SetSecondPassCallback(SecondWeakCallback);
481484
}
482485
}

‎samples/process.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -666,11 +666,13 @@ StringHttpRequest kSampleRequests[kSampleSize] = {
666666
};
667667

668668

669-
bool ProcessEntries(HttpRequestProcessor* processor, int count,
670-
StringHttpRequest* reqs) {
669+
bool ProcessEntries(v8::Platform* platform, HttpRequestProcessor* processor,
670+
int count, StringHttpRequest* reqs) {
671671
for (int i = 0; i < count; i++) {
672-
if (!processor->Process(&reqs[i]))
673-
return false;
672+
bool result = processor->Process(&reqs[i]);
673+
while (v8::platform::PumpMessageLoop(platform, Isolate::GetCurrent()))
674+
continue;
675+
if (!result) return false;
674676
}
675677
return true;
676678
}
@@ -713,7 +715,7 @@ int main(int argc, char* argv[]) {
713715
fprintf(stderr, "Error initializing processor.\n");
714716
return 1;
715717
}
716-
if (!ProcessEntries(&processor, kSampleSize, kSampleRequests))
718+
if (!ProcessEntries(platform, &processor, kSampleSize, kSampleRequests))
717719
return 1;
718720
PrintMap(&output);
719721
}

‎samples/shell.cc‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@
4545

4646

4747
v8::Local<v8::Context> CreateShellContext(v8::Isolate* isolate);
48-
void RunShell(v8::Local<v8::Context> context);
49-
int RunMain(v8::Isolate* isolate, int argc, char* argv[]);
48+
void RunShell(v8::Local<v8::Context> context, v8::Platform* platform);
49+
int RunMain(v8::Isolate* isolate, v8::Platform* platform, int argc,
50+
char* argv[]);
5051
bool ExecuteString(v8::Isolate* isolate, v8::Local<v8::String> source,
5152
v8::Local<v8::Value> name, bool print_result,
5253
bool report_exceptions);
@@ -94,8 +95,8 @@ int main(int argc, char* argv[]) {
9495
return 1;
9596
}
9697
v8::Context::Scope context_scope(context);
97-
result = RunMain(isolate, argc, argv);
98-
if (run_shell) RunShell(context);
98+
result = RunMain(isolate, platform, argc, argv);
99+
if (run_shell) RunShell(context, platform);
99100
}
100101
isolate->Dispose();
101102
v8::V8::Dispose();
@@ -269,7 +270,8 @@ v8::MaybeLocal<v8::String> ReadFile(v8::Isolate* isolate, const char* name) {
269270

270271

271272
// Process remaining command line arguments and execute files
272-
int RunMain(v8::Isolate* isolate, int argc, char* argv[]) {
273+
int RunMain(v8::Isolate* isolate, v8::Platform* platform, int argc,
274+
char* argv[]) {
273275
for (int i = 1; i < argc; i++) {
274276
const char* str = argv[i];
275277
if (strcmp(str, "--shell") == 0) {
@@ -292,7 +294,9 @@ int RunMain(v8::Isolate* isolate, int argc, char* argv[]) {
292294
.ToLocal(&source)) {
293295
return 1;
294296
}
295-
if (!ExecuteString(isolate, source, file_name, false, true)) return 1;
297+
bool success = ExecuteString(isolate, source, file_name, false, true);
298+
while (v8::platform::PumpMessageLoop(platform, isolate)) continue;
299+
if (!success) return 1;
296300
} else {
297301
// Use all other arguments as names of files to load and run.
298302
v8::Local<v8::String> file_name =
@@ -303,15 +307,17 @@ int RunMain(v8::Isolate* isolate, int argc, char* argv[]) {
303307
fprintf(stderr, "Error reading '%s'\n", str);
304308
continue;
305309
}
306-
if (!ExecuteString(isolate, source, file_name, false, true)) return 1;
310+
bool success = ExecuteString(isolate, source, file_name, false, true);
311+
while (v8::platform::PumpMessageLoop(platform, isolate)) continue;
312+
if (!success) return 1;
307313
}
308314
}
309315
return 0;
310316
}
311317

312318

313319
// The read-eval-execute loop of the shell.
314-
void RunShell(v8::Local<v8::Context> context) {
320+
void RunShell(v8::Local<v8::Context> context, v8::Platform* platform) {
315321
fprintf(stderr, "V8 version %s [sample shell]\n", v8::V8::GetVersion());
316322
static const int kBufferSize = 256;
317323
// Enter the execution environment before evaluating any code.
@@ -330,6 +336,8 @@ void RunShell(v8::Local<v8::Context> context) {
330336
v8::String::NewFromUtf8(context->GetIsolate(), str,
331337
v8::NewStringType::kNormal).ToLocalChecked(),
332338
name, true, true);
339+
while (v8::platform::PumpMessageLoop(platform, context->GetIsolate()))
340+
continue;
333341
}
334342
fprintf(stderr, "\n");
335343
}

‎src/d8.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ bool Shell::ExecuteString(Isolate* isolate, Handle<String> source,
349349
return false;
350350
}
351351
result = script->Run();
352+
EmptyMessageQueues(isolate);
352353
data->realm_current_ = data->realm_switch_;
353354
}
354355
if (result.IsEmpty()) {
@@ -2009,6 +2010,11 @@ void Shell::CollectGarbage(Isolate* isolate) {
20092010
}
20102011

20112012

2013+
void Shell::EmptyMessageQueues(Isolate* isolate) {
2014+
while (v8::platform::PumpMessageLoop(g_platform, isolate)) continue;
2015+
}
2016+
2017+
20122018
#ifndef V8_SHARED
20132019
bool Shell::SerializeValue(Isolate* isolate, Handle<Value> value,
20142020
const ObjectList& to_transfer,

‎src/d8.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ class Shell : public i::AllStatic {
366366
static void Exit(int exit_code);
367367
static void OnExit(Isolate* isolate);
368368
static void CollectGarbage(Isolate* isolate);
369+
static void EmptyMessageQueues(Isolate* isolate);
369370

370371
#ifndef V8_SHARED
371372
// TODO(binji): stupid implementation for now. Is there an easy way to hash an

‎src/global-handles.cc‎

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,29 @@ class GlobalHandles::NodeIterator {
495495
DISALLOW_COPY_AND_ASSIGN(NodeIterator);
496496
};
497497

498+
class GlobalHandles::PendingPhantomCallbacksSecondPassTask : public v8::Task {
499+
public:
500+
// Takes ownership of the contents of pending_phantom_callbacks, leaving it in
501+
// the same state it would be after a call to Clear().
502+
PendingPhantomCallbacksSecondPassTask(
503+
List<PendingPhantomCallback>* pending_phantom_callbacks, Isolate* isolate)
504+
: isolate_(isolate) {
505+
pending_phantom_callbacks_.Swap(pending_phantom_callbacks);
506+
}
507+
508+
~PendingPhantomCallbacksSecondPassTask() override {}
509+
510+
void Run() override {
511+
InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate_);
512+
}
513+
514+
private:
515+
List<PendingPhantomCallback> pending_phantom_callbacks_;
516+
Isolate* isolate_;
517+
518+
DISALLOW_COPY_AND_ASSIGN(PendingPhantomCallbacksSecondPassTask);
519+
};
520+
498521

499522
GlobalHandles::GlobalHandles(Isolate* isolate)
500523
: isolate_(isolate),
@@ -709,6 +732,19 @@ bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v,
709732
}
710733

711734

735+
void GlobalHandles::InvokeSecondPassPhantomCallbacks(
736+
List<PendingPhantomCallback>* callbacks, Isolate* isolate) {
737+
while (callbacks->length() != 0) {
738+
auto callback = callbacks->RemoveLast();
739+
DCHECK(callback.node() == nullptr);
740+
// No second pass callback required.
741+
if (callback.callback() == nullptr) continue;
742+
// Fire second pass callback
743+
callback.Invoke(isolate);
744+
}
745+
}
746+
747+
712748
int GlobalHandles::PostScavengeProcessing(
713749
const int initial_post_gc_processing_count) {
714750
int freed_nodes = 0;
@@ -791,7 +827,8 @@ void GlobalHandles::UpdateListOfNewSpaceNodes() {
791827
}
792828

793829

794-
int GlobalHandles::DispatchPendingPhantomCallbacks() {
830+
int GlobalHandles::DispatchPendingPhantomCallbacks(
831+
bool synchronous_second_pass) {
795832
int freed_nodes = 0;
796833
{
797834
// The initial pass callbacks must simply clear the nodes.
@@ -804,14 +841,15 @@ int GlobalHandles::DispatchPendingPhantomCallbacks() {
804841
freed_nodes++;
805842
}
806843
}
807-
// The second pass empties the list.
808-
while (pending_phantom_callbacks_.length() != 0) {
809-
auto callback = pending_phantom_callbacks_.RemoveLast();
810-
DCHECK(callback.node() == nullptr);
811-
// No second pass callback required.
812-
if (callback.callback() == nullptr) continue;
813-
// Fire second pass callback.
814-
callback.Invoke(isolate());
844+
if (pending_phantom_callbacks_.length() > 0) {
845+
if (synchronous_second_pass) {
846+
InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate());
847+
} else {
848+
auto* task = new PendingPhantomCallbacksSecondPassTask(
849+
&pending_phantom_callbacks_, isolate());
850+
V8::GetCurrentPlatform()->CallOnForegroundThread(
851+
reinterpret_cast<v8::Isolate*>(isolate()), task);
852+
}
815853
}
816854
pending_phantom_callbacks_.Clear();
817855
return freed_nodes;
@@ -838,14 +876,17 @@ void GlobalHandles::PendingPhantomCallback::Invoke(Isolate* isolate) {
838876
}
839877

840878

841-
int GlobalHandles::PostGarbageCollectionProcessing(GarbageCollector collector) {
879+
int GlobalHandles::PostGarbageCollectionProcessing(
880+
GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags) {
842881
// Process weak global handle callbacks. This must be done after the
843882
// GC is completely done, because the callbacks may invoke arbitrary
844883
// API functions.
845884
DCHECK(isolate_->heap()->gc_state() == Heap::NOT_IN_GC);
846885
const int initial_post_gc_processing_count = ++post_gc_processing_count_;
847886
int freed_nodes = 0;
848-
freed_nodes += DispatchPendingPhantomCallbacks();
887+
bool synchronous_second_pass =
888+
(gc_callback_flags & kGCCallbackFlagForced) != 0;
889+
freed_nodes += DispatchPendingPhantomCallbacks(synchronous_second_pass);
849890
if (initial_post_gc_processing_count != post_gc_processing_count_) {
850891
// If the callbacks caused a nested GC, then return. See comment in
851892
// PostScavengeProcessing.

‎src/global-handles.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ class GlobalHandles {
181181

182182
// Process pending weak handles.
183183
// Returns the number of freed nodes.
184-
int PostGarbageCollectionProcessing(GarbageCollector collector);
184+
int PostGarbageCollectionProcessing(
185+
GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags);
185186

186187
// Iterates over all strong handles.
187188
void IterateStrongRoots(ObjectVisitor* v);
@@ -287,17 +288,21 @@ class GlobalHandles {
287288
// don't assign any initial capacity.
288289
static const int kObjectGroupConnectionsCapacity = 20;
289290

291+
class PendingPhantomCallback;
292+
290293
// Helpers for PostGarbageCollectionProcessing.
294+
static void InvokeSecondPassPhantomCallbacks(
295+
List<PendingPhantomCallback>* callbacks, Isolate* isolate);
291296
int PostScavengeProcessing(int initial_post_gc_processing_count);
292297
int PostMarkSweepProcessing(int initial_post_gc_processing_count);
293-
int DispatchPendingPhantomCallbacks();
298+
int DispatchPendingPhantomCallbacks(bool synchronous_second_pass);
294299
void UpdateListOfNewSpaceNodes();
295300

296301
// Internal node structures.
297302
class Node;
298303
class NodeBlock;
299304
class NodeIterator;
300-
class PendingPhantomCallback;
305+
class PendingPhantomCallbacksSecondPassTask;
301306

302307
Isolate* isolate_;
303308

‎src/heap/heap.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,8 @@ bool Heap::PerformGarbageCollection(
13061306
AllowHeapAllocation allow_allocation;
13071307
GCTracer::Scope scope(tracer(), GCTracer::Scope::EXTERNAL);
13081308
freed_global_handles =
1309-
isolate_->global_handles()->PostGarbageCollectionProcessing(collector);
1309+
isolate_->global_handles()->PostGarbageCollectionProcessing(
1310+
collector, gc_callback_flags);
13101311
}
13111312
gc_post_processing_depth_--;
13121313

‎src/list-inl.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ bool List<T, P>::RemoveElement(const T& elm) {
125125
return false;
126126
}
127127

128+
template <typename T, class P>
129+
void List<T, P>::Swap(List<T, P>* list) {
130+
std::swap(data_, list->data_);
131+
std::swap(length_, list->length_);
132+
std::swap(capacity_, list->capacity_);
133+
}
128134

129135
template<typename T, class P>
130136
void List<T, P>::Allocate(int length, P allocator) {

‎src/list.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#ifndef V8_LIST_H_
66
#define V8_LIST_H_
77

8+
#include <algorithm>
9+
810
#include "src/checks.h"
911
#include "src/utils.h"
1012

@@ -137,6 +139,9 @@ class List {
137139
// Drop the last 'count' elements from the list.
138140
INLINE(void RewindBy(int count)) { Rewind(length_ - count); }
139141

142+
// Swaps the contents of the two lists.
143+
INLINE(void Swap(List<T, AllocationPolicy>* list));
144+
140145
// Halve the capacity if fill level is less than a quarter.
141146
INLINE(void Trim(AllocationPolicy allocator = AllocationPolicy()));
142147

0 commit comments

Comments
 (0)