Skip to content

Commit 02595c6

Browse files
mi-acCommit Bot
mi-ac
authored and
Commit Bot
committed
Revert "Revert "Revert "Introducing an event loop mechanism for d8."""
This reverts commit 7dcc8ef. Reason for revert: Some flakes still (see comments) and breaks predictable testing: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20predictable/builds/11452 Original change's description: > Revert "Revert "Introducing an event loop mechanism for d8."" > > This reverts commit f7c25da. > > Reason for revert: Fixed > > Original change's description: > > Revert "Introducing an event loop mechanism for d8." > > > > This reverts commit de964db. > > > > Reason for revert: > > https://build.chromium.org/p/client.v8/builders/V8%20Linux/builds/17958 > > > > Original change's description: > > > Introducing an event loop mechanism for d8. > > > > > > This mechanism ensures APIs like wasm async complete their work, > > > without requiring use of natives (%APIs). > > > > > > The mechanism is similar to the one used in content_shell, > > > which should allow us to easily port tests in that environment. > > > > > > Review-Url: https://codereview.chromium.org/2842843005 > > > Cr-Original-Commit-Position: refs/heads/master@{#44908} > > > Bug: > > > Change-Id: I9deee0d256a600c60b42902fc8ef8478e5546344 > > > Reviewed-on: https://chromium-review.googlesource.com/494968 > > > Commit-Queue: Mircea Trofin <mtrofin@google.com> > > > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > > > Cr-Commit-Position: refs/heads/master@{#45165} > > > > TBR=bradnelson@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > > > Change-Id: Iafec2615d705d1990c57229cab3a988c00b5e12f > > Reviewed-on: https://chromium-review.googlesource.com/498630 > > Reviewed-by: Michael Achenbach <machenbach@chromium.org> > > Commit-Queue: Michael Achenbach <machenbach@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#45166} > > TBR=bradnelson@chromium.org,machenbach@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org,v8-reviews@googlegroups.com > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > > Change-Id: Ic3c782e918326e291a6cb9bb349c609e9a340b09 > Reviewed-on: https://chromium-review.googlesource.com/498430 > Reviewed-by: Mircea Trofin <mtrofin@chromium.org> > Commit-Queue: Mircea Trofin <mtrofin@google.com> > Cr-Commit-Position: refs/heads/master@{#45172} TBR=bradnelson@chromium.org,machenbach@chromium.org,mtrofin@chromium.org,mtrofin@google.com,jochen@chromium.org,v8-reviews@googlegroups.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Change-Id: I21ffba7141db0bfb4a3275b6e1bf4fb399800ed2 Reviewed-on: https://chromium-review.googlesource.com/500128 Reviewed-by: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#45177}
1 parent ab341ea commit 02595c6

15 files changed

Lines changed: 89 additions & 152 deletions

‎include/libplatform/libplatform.h‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ namespace platform {
1515
enum class IdleTaskSupport { kDisabled, kEnabled };
1616
enum class InProcessStackDumping { kDisabled, kEnabled };
1717

18-
enum class MessageLoopBehavior : bool {
19-
kDoNotWait = false,
20-
kWaitForWork = true
21-
};
22-
2318
/**
2419
* Returns a new instance of the default v8::Platform implementation.
2520
*
@@ -41,16 +36,12 @@ V8_PLATFORM_EXPORT v8::Platform* CreateDefaultPlatform(
4136
* Pumps the message loop for the given isolate.
4237
*
4338
* The caller has to make sure that this is called from the right thread.
44-
* Returns true if a task was executed, and false otherwise. Unless requested
45-
* through the |behavior| parameter, this call does not block if no task is
46-
* pending. The |platform| has to be created using |CreateDefaultPlatform|.
39+
* Returns true if a task was executed, and false otherwise. This call does
40+
* not block if no task is pending. The |platform| has to be created using
41+
* |CreateDefaultPlatform|.
4742
*/
48-
V8_PLATFORM_EXPORT bool PumpMessageLoop(
49-
v8::Platform* platform, v8::Isolate* isolate,
50-
MessageLoopBehavior behavior = MessageLoopBehavior::kDoNotWait);
51-
52-
V8_PLATFORM_EXPORT void EnsureEventLoopInitialized(v8::Platform* platform,
53-
v8::Isolate* isolate);
43+
V8_PLATFORM_EXPORT bool PumpMessageLoop(v8::Platform* platform,
44+
v8::Isolate* isolate);
5445

5546
/**
5647
* Runs pending idle tasks for at most |idle_time_in_seconds| seconds.

‎src/d8.cc‎

Lines changed: 16 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,6 @@ base::LazyMutex Shell::workers_mutex_;
422422
bool Shell::allow_new_workers_ = true;
423423
i::List<Worker*> Shell::workers_;
424424
std::vector<ExternalizedContents> Shell::externalized_contents_;
425-
std::map<v8::Isolate*, bool> Shell::isolate_status;
426425

427426
Global<Context> Shell::evaluation_context_;
428427
ArrayBuffer::Allocator* Shell::array_buffer_allocator;
@@ -1346,18 +1345,6 @@ void Shell::Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
13461345
const_cast<v8::FunctionCallbackInfo<v8::Value>*>(&args));
13471346
}
13481347

1349-
void Shell::WaitUntilDone(const v8::FunctionCallbackInfo<v8::Value>& args) {
1350-
if (isolate_status.count(args.GetIsolate()) == 0) {
1351-
isolate_status.insert(std::make_pair(args.GetIsolate(), true));
1352-
} else {
1353-
isolate_status[args.GetIsolate()] = true;
1354-
}
1355-
}
1356-
1357-
void Shell::NotifyDone(const v8::FunctionCallbackInfo<v8::Value>& args) {
1358-
DCHECK_EQ(isolate_status.count(args.GetIsolate()), 1);
1359-
isolate_status[args.GetIsolate()] = false;
1360-
}
13611348

13621349
void Shell::Version(const v8::FunctionCallbackInfo<v8::Value>& args) {
13631350
args.GetReturnValue().Set(
@@ -1595,19 +1582,6 @@ Local<ObjectTemplate> Shell::CreateGlobalTemplate(Isolate* isolate) {
15951582
.ToLocalChecked(),
15961583
FunctionTemplate::New(isolate, Quit));
15971584
}
1598-
Local<ObjectTemplate> test_template = ObjectTemplate::New(isolate);
1599-
global_template->Set(
1600-
String::NewFromUtf8(isolate, "testRunner", NewStringType::kNormal)
1601-
.ToLocalChecked(),
1602-
test_template);
1603-
test_template->Set(
1604-
String::NewFromUtf8(isolate, "notifyDone", NewStringType::kNormal)
1605-
.ToLocalChecked(),
1606-
FunctionTemplate::New(isolate, NotifyDone));
1607-
test_template->Set(
1608-
String::NewFromUtf8(isolate, "waitUntilDone", NewStringType::kNormal)
1609-
.ToLocalChecked(),
1610-
FunctionTemplate::New(isolate, WaitUntilDone));
16111585
global_template->Set(
16121586
String::NewFromUtf8(isolate, "version", NewStringType::kNormal)
16131587
.ToLocalChecked(),
@@ -2292,8 +2266,6 @@ void SourceGroup::ExecuteInThread() {
22922266
create_params.host_import_module_dynamically_callback_ =
22932267
Shell::HostImportModuleDynamically;
22942268
Isolate* isolate = Isolate::New(create_params);
2295-
2296-
v8::platform::EnsureEventLoopInitialized(g_platform, isolate);
22972269
D8Console console(isolate);
22982270
debug::SetConsoleDelegate(isolate, &console);
22992271
for (int i = 0; i < Shell::options.stress_runs; ++i) {
@@ -2314,7 +2286,6 @@ void SourceGroup::ExecuteInThread() {
23142286
DisposeModuleEmbedderData(context);
23152287
}
23162288
Shell::CollectGarbage(isolate);
2317-
Shell::CompleteMessageLoop(isolate);
23182289
}
23192290
done_semaphore_.Signal();
23202291
}
@@ -2675,7 +2646,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
26752646
options.isolate_sources[i].StartExecuteInThread();
26762647
}
26772648
{
2678-
v8::platform::EnsureEventLoopInitialized(g_platform, isolate);
26792649
if (options.lcov_file) {
26802650
debug::Coverage::SelectMode(isolate, debug::Coverage::kPreciseCount);
26812651
}
@@ -2698,7 +2668,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
26982668
WriteLcovData(isolate, options.lcov_file);
26992669
}
27002670
CollectGarbage(isolate);
2701-
CompleteMessageLoop(isolate);
27022671
for (int i = 1; i < options.num_isolates; ++i) {
27032672
if (last_run) {
27042673
options.isolate_sources[i].JoinThread();
@@ -2726,28 +2695,24 @@ void Shell::CollectGarbage(Isolate* isolate) {
27262695
}
27272696
}
27282697

2729-
void Shell::CompleteMessageLoop(Isolate* isolate) {
2730-
while (v8::platform::PumpMessageLoop(
2731-
g_platform, isolate,
2732-
Shell::isolate_status[isolate]
2733-
? platform::MessageLoopBehavior::kWaitForWork
2734-
: platform::MessageLoopBehavior::kDoNotWait)) {
2735-
isolate->RunMicrotasks();
2736-
}
2737-
v8::platform::RunIdleTasks(g_platform, isolate,
2738-
50.0 / base::Time::kMillisecondsPerSecond);
2739-
}
2740-
27412698
void Shell::EmptyMessageQueues(Isolate* isolate) {
27422699
if (i::FLAG_verify_predictable) return;
2743-
// Pump the message loop until it is empty.
2744-
while (v8::platform::PumpMessageLoop(
2745-
g_platform, isolate, platform::MessageLoopBehavior::kDoNotWait)) {
2746-
isolate->RunMicrotasks();
2747-
}
2748-
// Run the idle tasks.
2749-
v8::platform::RunIdleTasks(g_platform, isolate,
2750-
50.0 / base::Time::kMillisecondsPerSecond);
2700+
while (true) {
2701+
// Pump the message loop until it is empty.
2702+
while (v8::platform::PumpMessageLoop(g_platform, isolate)) {
2703+
isolate->RunMicrotasks();
2704+
}
2705+
// Run the idle tasks.
2706+
v8::platform::RunIdleTasks(g_platform, isolate,
2707+
50.0 / base::Time::kMillisecondsPerSecond);
2708+
// If there are still outstanding waiters, sleep a little (to wait for
2709+
// background tasks) and then try everything again.
2710+
if (reinterpret_cast<i::Isolate*>(isolate)->GetWaitCountForTesting() > 0) {
2711+
base::OS::Sleep(base::TimeDelta::FromMilliseconds(1));
2712+
} else {
2713+
break;
2714+
}
2715+
}
27512716
}
27522717

27532718
class Serializer : public ValueSerializer::Delegate {

‎src/d8.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#define V8_D8_H_
77

88
#include <iterator>
9-
#include <map>
109
#include <memory>
1110
#include <string>
1211
#include <vector>
@@ -357,7 +356,6 @@ class Shell : public i::AllStatic {
357356
static void OnExit(Isolate* isolate);
358357
static void CollectGarbage(Isolate* isolate);
359358
static void EmptyMessageQueues(Isolate* isolate);
360-
static void CompleteMessageLoop(Isolate* isolate);
361359

362360
static std::unique_ptr<SerializationData> SerializeValue(
363361
Isolate* isolate, Local<Value> value, Local<Value> transfer);
@@ -393,8 +391,6 @@ class Shell : public i::AllStatic {
393391
static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
394392
static void PrintErr(const v8::FunctionCallbackInfo<v8::Value>& args);
395393
static void Write(const v8::FunctionCallbackInfo<v8::Value>& args);
396-
static void WaitUntilDone(const v8::FunctionCallbackInfo<v8::Value>& args);
397-
static void NotifyDone(const v8::FunctionCallbackInfo<v8::Value>& args);
398394
static void QuitOnce(v8::FunctionCallbackInfo<v8::Value>* args);
399395
static void Quit(const v8::FunctionCallbackInfo<v8::Value>& args);
400396
static void Version(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -458,7 +454,6 @@ class Shell : public i::AllStatic {
458454
static const char* kPrompt;
459455
static ShellOptions options;
460456
static ArrayBuffer::Allocator* array_buffer_allocator;
461-
static std::map<Isolate*, bool> isolate_status;
462457

463458
private:
464459
static Global<Context> evaluation_context_;

‎src/isolate.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,6 +1291,11 @@ class Isolate {
12911291
// reset to nullptr.
12921292
void UnregisterFromReleaseAtTeardown(ManagedObjectFinalizer** finalizer_ptr);
12931293

1294+
// Used by mjsunit tests to force d8 to wait for certain things to run.
1295+
inline void IncrementWaitCountForTesting() { wait_count_++; }
1296+
inline void DecrementWaitCountForTesting() { wait_count_--; }
1297+
inline int GetWaitCountForTesting() { return wait_count_; }
1298+
12941299
protected:
12951300
explicit Isolate(bool enable_serializer);
12961301
bool IsArrayOrObjectPrototype(Object* object);
@@ -1577,6 +1582,8 @@ class Isolate {
15771582

15781583
size_t total_regexp_code_generated_;
15791584

1585+
int wait_count_ = 0;
1586+
15801587
friend class ExecutionAccess;
15811588
friend class HandleScopeImplementer;
15821589
friend class HeapTester;

‎src/libplatform/default-platform.cc‎

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,9 @@ v8::Platform* CreateDefaultPlatform(
4141
return platform;
4242
}
4343

44-
bool PumpMessageLoop(v8::Platform* platform, v8::Isolate* isolate,
45-
MessageLoopBehavior behavior) {
46-
return reinterpret_cast<DefaultPlatform*>(platform)->PumpMessageLoop(
47-
isolate, behavior);
48-
}
4944

50-
void EnsureEventLoopInitialized(v8::Platform* platform, v8::Isolate* isolate) {
51-
return reinterpret_cast<DefaultPlatform*>(platform)
52-
->EnsureEventLoopInitialized(isolate);
45+
bool PumpMessageLoop(v8::Platform* platform, v8::Isolate* isolate) {
46+
return reinterpret_cast<DefaultPlatform*>(platform)->PumpMessageLoop(isolate);
5347
}
5448

5549
void RunIdleTasks(v8::Platform* platform, v8::Isolate* isolate,
@@ -164,38 +158,22 @@ IdleTask* DefaultPlatform::PopTaskInMainThreadIdleQueue(v8::Isolate* isolate) {
164158
return task;
165159
}
166160

167-
void DefaultPlatform::EnsureEventLoopInitialized(v8::Isolate* isolate) {
168-
if (event_loop_control_.count(isolate) == 0) {
169-
event_loop_control_.insert(std::make_pair(
170-
isolate, std::unique_ptr<base::Semaphore>(new base::Semaphore(0))));
171-
}
172-
}
173-
174-
void DefaultPlatform::WaitForForegroundWork(v8::Isolate* isolate) {
175-
DCHECK_EQ(event_loop_control_.count(isolate), 1);
176-
event_loop_control_[isolate]->Wait();
177-
}
178-
179-
bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate,
180-
MessageLoopBehavior behavior) {
181-
if (behavior == MessageLoopBehavior::kWaitForWork) {
182-
WaitForForegroundWork(isolate);
183-
}
161+
bool DefaultPlatform::PumpMessageLoop(v8::Isolate* isolate) {
184162
Task* task = NULL;
185163
{
186164
base::LockGuard<base::Mutex> guard(&lock_);
187165

188166
// Move delayed tasks that hit their deadline to the main queue.
189167
task = PopTaskInMainThreadDelayedQueue(isolate);
190168
while (task != NULL) {
191-
ScheduleOnForegroundThread(isolate, task);
169+
main_thread_queue_[isolate].push(task);
192170
task = PopTaskInMainThreadDelayedQueue(isolate);
193171
}
194172

195173
task = PopTaskInMainThreadQueue(isolate);
196174

197175
if (task == NULL) {
198-
return behavior == MessageLoopBehavior::kWaitForWork;
176+
return false;
199177
}
200178
}
201179
task->Run();
@@ -228,17 +206,10 @@ void DefaultPlatform::CallOnBackgroundThread(Task* task,
228206
queue_.Append(task);
229207
}
230208

231-
void DefaultPlatform::ScheduleOnForegroundThread(v8::Isolate* isolate,
232-
Task* task) {
233-
main_thread_queue_[isolate].push(task);
234-
if (event_loop_control_.count(isolate) != 0) {
235-
event_loop_control_[isolate]->Signal();
236-
}
237-
}
238209

239210
void DefaultPlatform::CallOnForegroundThread(v8::Isolate* isolate, Task* task) {
240211
base::LockGuard<base::Mutex> guard(&lock_);
241-
ScheduleOnForegroundThread(isolate, task);
212+
main_thread_queue_[isolate].push(task);
242213
}
243214

244215

‎src/libplatform/default-platform.h‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
4141

4242
void EnsureInitialized();
4343

44-
bool PumpMessageLoop(
45-
v8::Isolate* isolate,
46-
MessageLoopBehavior behavior = MessageLoopBehavior::kDoNotWait);
47-
void EnsureEventLoopInitialized(v8::Isolate* isolate);
44+
bool PumpMessageLoop(v8::Isolate* isolate);
4845

4946
void RunIdleTasks(v8::Isolate* isolate, double idle_time_in_seconds);
5047

@@ -84,9 +81,6 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
8481
Task* PopTaskInMainThreadDelayedQueue(v8::Isolate* isolate);
8582
IdleTask* PopTaskInMainThreadIdleQueue(v8::Isolate* isolate);
8683

87-
void WaitForForegroundWork(v8::Isolate* isolate);
88-
void ScheduleOnForegroundThread(v8::Isolate* isolate, Task* task);
89-
9084
base::Mutex lock_;
9185
bool initialized_;
9286
int thread_pool_size_;
@@ -95,7 +89,6 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) {
9589
TaskQueue queue_;
9690
std::map<v8::Isolate*, std::queue<Task*>> main_thread_queue_;
9791
std::map<v8::Isolate*, std::queue<IdleTask*>> main_thread_idle_queue_;
98-
std::map<v8::Isolate*, std::unique_ptr<base::Semaphore>> event_loop_control_;
9992

10093
typedef std::pair<double, Task*> DelayedEntry;
10194
std::map<v8::Isolate*,

‎src/runtime/runtime-test.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,5 +1019,15 @@ RUNTIME_FUNCTION(Runtime_RedirectToWasmInterpreter) {
10191019
return isolate->heap()->undefined_value();
10201020
}
10211021

1022+
RUNTIME_FUNCTION(Runtime_IncrementWaitCount) {
1023+
isolate->IncrementWaitCountForTesting();
1024+
return isolate->heap()->undefined_value();
1025+
}
1026+
1027+
RUNTIME_FUNCTION(Runtime_DecrementWaitCount) {
1028+
isolate->DecrementWaitCountForTesting();
1029+
return isolate->heap()->undefined_value();
1030+
}
1031+
10221032
} // namespace internal
10231033
} // namespace v8

‎src/runtime/runtime.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,9 @@ namespace internal {
471471
F(PromiseRevokeReject, 1, 1) \
472472
F(PromiseResult, 1, 1) \
473473
F(PromiseStatus, 1, 1) \
474-
F(ReportPromiseReject, 2, 1)
474+
F(ReportPromiseReject, 2, 1) \
475+
F(IncrementWaitCount, 0, 1) \
476+
F(DecrementWaitCount, 0, 1)
475477

476478
#define FOR_EACH_INTRINSIC_PROXY(F) \
477479
F(IsJSProxy, 1, 1) \

‎test/debugger/debug/debug-stepin-accessor.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ function testProtoSetter1_2() {
233233
}
234234

235235
for (var n in this) {
236-
if (n.substr(0, 4) != 'test' ||
237-
n == 'testRunner') {
236+
if (n.substr(0, 4) != 'test') {
238237
continue;
239238
}
240239
state = 1;

‎test/mjsunit/basic-promise.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,22 @@
88
// exceptions which are swallowed in a then clause.
99
failWithMessage = (msg) => %AbortJS(msg);
1010

11+
let decrement = () => { %DecrementWaitCount(); }
12+
let increment = () => { %IncrementWaitCount(); }
13+
14+
function WaitForPromise(p) {
15+
increment();
16+
p.then(decrement, decrement);
17+
}
18+
1119
function newPromise() {
1220
var outerResolve;
1321
var outerReject;
1422
let promise = new Promise((resolve, reject) => {
1523
outerResolve = resolve;
1624
outerReject = reject;
1725
});
18-
Promise.resolve(promise);
26+
WaitForPromise(promise); // explicitly wait for promise to resolve.
1927
return {
2028
resolve: outerResolve,
2129
reject: outerReject,

0 commit comments

Comments
 (0)