From 662dd448750b767e89761f3bf28f29e24b17179e Mon Sep 17 00:00:00 2001 From: "jochen@chromium.org" Date: Thu, 21 Nov 2013 13:47:37 +0000 Subject: [PATCH] Remove preemption thread and API BUG=v8:3004 R=svenpanne@chromium.org, yangguo@chromium.org LOG=y Review URL: https://codereview.chromium.org/62283010 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17967 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 6 --- src/d8.cc | 50 -------------------- src/d8.h | 4 -- src/execution.cc | 2 - src/flag-definitions.h | 4 -- src/isolate.cc | 12 ----- src/isolate.h | 8 ---- src/v8threads.cc | 70 ---------------------------- src/v8threads.h | 28 ----------- test/cctest/test-api.cc | 110 -------------------------------------------- test/cctest/test-threads.cc | 22 --------- 11 files changed, 316 deletions(-) diff --git a/include/v8.h b/include/v8.h index 6e227dd..bb0c90a 100644 --- a/include/v8.h +++ b/include/v8.h @@ -5271,12 +5271,6 @@ class V8_EXPORT Locker { ~Locker(); - V8_DEPRECATED("This will be remvoed.", - static void StartPreemption(Isolate *isolate, int every_n_ms)); - - V8_DEPRECATED("This will be removed", - static void StopPreemption(Isolate* isolate)); - /** * Returns whether or not the locker for a given isolate, is locked by the * current thread. diff --git a/src/d8.cc b/src/d8.cc index 5b128c0..41b08bc 100644 --- a/src/d8.cc +++ b/src/d8.cc @@ -1379,43 +1379,6 @@ bool Shell::SetOptions(int argc, char* argv[]) { } else if (strcmp(argv[i], "--send-idle-notification") == 0) { options.send_idle_notification = true; argv[i] = NULL; - } else if (strcmp(argv[i], "--preemption") == 0) { -#ifdef V8_SHARED - printf("D8 with shared library does not support multi-threading\n"); - return false; -#else - options.use_preemption = true; - argv[i] = NULL; -#endif // V8_SHARED - } else if (strcmp(argv[i], "--nopreemption") == 0) { -#ifdef V8_SHARED - printf("D8 with shared library does not support multi-threading\n"); - return false; -#else - options.use_preemption = false; - argv[i] = NULL; -#endif // V8_SHARED - } else if (strcmp(argv[i], "--preemption-interval") == 0) { -#ifdef V8_SHARED - printf("D8 with shared library does not support multi-threading\n"); - return false; -#else - if (++i < argc) { - argv[i-1] = NULL; - char* end = NULL; - options.preemption_interval = strtol(argv[i], &end, 10); // NOLINT - if (options.preemption_interval <= 0 - || *end != '\0' - || errno == ERANGE) { - printf("Invalid value for --preemption-interval '%s'\n", argv[i]); - return false; - } - argv[i] = NULL; - } else { - printf("Missing value for --preemption-interval\n"); - return false; - } -#endif // V8_SHARED } else if (strcmp(argv[i], "-f") == 0) { // Ignore any -f flags for compatibility with other stand-alone // JavaScript engines. @@ -1554,14 +1517,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) { V8::IdleNotification(kLongIdlePauseInMs); } } - -#ifndef V8_SHARED - // Start preemption if threads have been created and preemption is enabled. - if (threads.length() > 0 - && options.use_preemption) { - Locker::StartPreemption(isolate, options.preemption_interval); - } -#endif // V8_SHARED } #ifndef V8_SHARED @@ -1574,11 +1529,6 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) { thread->Join(); delete thread; } - - if (threads.length() > 0 && options.use_preemption) { - Locker lock(isolate); - Locker::StopPreemption(isolate); - } #endif // V8_SHARED return 0; } diff --git a/src/d8.h b/src/d8.h index 8c1687e..761276e 100644 --- a/src/d8.h +++ b/src/d8.h @@ -219,8 +219,6 @@ class ShellOptions { public: ShellOptions() : #ifndef V8_SHARED - use_preemption(true), - preemption_interval(10), num_parallel_files(0), parallel_files(NULL), #endif // V8_SHARED @@ -245,8 +243,6 @@ class ShellOptions { } #ifndef V8_SHARED - bool use_preemption; - int preemption_interval; int num_parallel_files; char** parallel_files; #endif // V8_SHARED diff --git a/src/execution.cc b/src/execution.cc index 8febbbf..32c616f 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -814,8 +814,6 @@ static Object* RuntimePreempt(Isolate* isolate) { // Clear the preempt request flag. isolate->stack_guard()->Continue(PREEMPT); - ContextSwitcher::PreemptionReceived(); - #ifdef ENABLE_DEBUGGER_SUPPORT if (isolate->debug()->InDebugger()) { // If currently in the debugger don't do any actual preemption but record diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 62cb307..1c95885 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -611,10 +611,6 @@ DEFINE_int(hash_seed, 0, DEFINE_bool(profile_deserialization, false, "Print the time it takes to deserialize the snapshot.") -// v8.cc -DEFINE_bool(preemption, false, - "activate a 100ms timer that switches between V8 threads") - // Regexp DEFINE_bool(regexp_optimization, true, "generate optimized regexp code") diff --git a/src/isolate.cc b/src/isolate.cc index 7250246..2acc59a 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -1549,7 +1549,6 @@ Isolate::Isolate() write_iterator_(NULL), global_handles_(NULL), eternal_handles_(NULL), - context_switcher_(NULL), thread_manager_(NULL), fp_stubs_generated_(false), has_installed_extensions_(false), @@ -1690,10 +1689,6 @@ void Isolate::Deinit() { delete deoptimizer_data_; deoptimizer_data_ = NULL; - if (FLAG_preemption) { - v8::Locker locker(reinterpret_cast(this)); - v8::Locker::StopPreemption(reinterpret_cast(this)); - } builtins_.TearDown(); bootstrapper_->TearDown(); @@ -1805,8 +1800,6 @@ Isolate::~Isolate() { delete write_iterator_; write_iterator_ = NULL; - delete context_switcher_; - context_switcher_ = NULL; delete thread_manager_; thread_manager_ = NULL; @@ -2038,11 +2031,6 @@ bool Isolate::Init(Deserializer* des) { } } - if (FLAG_preemption) { - v8::Locker locker(reinterpret_cast(this)); - v8::Locker::StartPreemption(reinterpret_cast(this), 100); - } - #ifdef ENABLE_DEBUGGER_SUPPORT debug_->SetUp(create_heap_objects); #endif diff --git a/src/isolate.h b/src/isolate.h index ed568b7..f280ee8 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -58,7 +58,6 @@ struct CodeStubInterfaceDescriptor; class CodeTracer; class CompilationCache; class ContextSlotCache; -class ContextSwitcher; class Counters; class CpuFeatures; class CpuProfiler; @@ -919,12 +918,6 @@ class Isolate { ThreadManager* thread_manager() { return thread_manager_; } - ContextSwitcher* context_switcher() { return context_switcher_; } - - void set_context_switcher(ContextSwitcher* switcher) { - context_switcher_ = switcher; - } - StringTracker* string_tracker() { return string_tracker_; } unibrow::Mapping* jsregexp_uncanonicalize() { @@ -1293,7 +1286,6 @@ class Isolate { ConsStringIteratorOp* write_iterator_; GlobalHandles* global_handles_; EternalHandles* eternal_handles_; - ContextSwitcher* context_switcher_; ThreadManager* thread_manager_; RuntimeState runtime_state_; bool fp_stubs_generated_; diff --git a/src/v8threads.cc b/src/v8threads.cc index cc4f439..1de9d4f 100644 --- a/src/v8threads.cc +++ b/src/v8threads.cc @@ -133,18 +133,6 @@ Unlocker::~Unlocker() { } -void Locker::StartPreemption(v8::Isolate* isolate, int every_n_ms) { - v8::internal::ContextSwitcher::StartPreemption( - reinterpret_cast(isolate), every_n_ms); -} - - -void Locker::StopPreemption(v8::Isolate* isolate) { - v8::internal::ContextSwitcher::StopPreemption( - reinterpret_cast(isolate)); -} - - namespace internal { @@ -419,63 +407,5 @@ void ThreadManager::TerminateExecution(ThreadId thread_id) { } -ContextSwitcher::ContextSwitcher(Isolate* isolate, int every_n_ms) - : Thread("v8:CtxtSwitcher"), - keep_going_(true), - sleep_ms_(every_n_ms), - isolate_(isolate) { -} - - -// Set the scheduling interval of V8 threads. This function starts the -// ContextSwitcher thread if needed. -void ContextSwitcher::StartPreemption(Isolate* isolate, int every_n_ms) { - ASSERT(Locker::IsLocked(reinterpret_cast(isolate))); - if (isolate->context_switcher() == NULL) { - // If the ContextSwitcher thread is not running at the moment start it now. - isolate->set_context_switcher(new ContextSwitcher(isolate, every_n_ms)); - isolate->context_switcher()->Start(); - } else { - // ContextSwitcher thread is already running, so we just change the - // scheduling interval. - isolate->context_switcher()->sleep_ms_ = every_n_ms; - } -} - - -// Disable preemption of V8 threads. If multiple threads want to use V8 they -// must cooperatively schedule amongst them from this point on. -void ContextSwitcher::StopPreemption(Isolate* isolate) { - ASSERT(Locker::IsLocked(reinterpret_cast(isolate))); - if (isolate->context_switcher() != NULL) { - // The ContextSwitcher thread is running. We need to stop it and release - // its resources. - isolate->context_switcher()->keep_going_ = false; - // Wait for the ContextSwitcher thread to exit. - isolate->context_switcher()->Join(); - // Thread has exited, now we can delete it. - delete(isolate->context_switcher()); - isolate->set_context_switcher(NULL); - } -} - - -// Main loop of the ContextSwitcher thread: Preempt the currently running V8 -// thread at regular intervals. -void ContextSwitcher::Run() { - while (keep_going_) { - OS::Sleep(sleep_ms_); - isolate()->stack_guard()->Preempt(); - } -} - - -// Acknowledge the preemption by the receiving thread. -void ContextSwitcher::PreemptionReceived() { - // There is currently no accounting being done for this. But could be in the - // future, which is why we leave this in. -} - - } // namespace internal } // namespace v8 diff --git a/src/v8threads.h b/src/v8threads.h index 1edacfc..a20700a 100644 --- a/src/v8threads.h +++ b/src/v8threads.h @@ -139,34 +139,6 @@ class ThreadManager { }; -// The ContextSwitcher thread is used to schedule regular preemptions to -// multiple running V8 threads. Generally it is necessary to call -// StartPreemption if there is more than one thread running. If not, a single -// JavaScript can take full control of V8 and not allow other threads to run. -class ContextSwitcher: public Thread { - public: - // Set the preemption interval for the ContextSwitcher thread. - static void StartPreemption(Isolate* isolate, int every_n_ms); - - // Stop sending preemption requests to threads. - static void StopPreemption(Isolate* isolate); - - // Preempted thread needs to call back to the ContextSwitcher to acknowledge - // the handling of a preemption request. - static void PreemptionReceived(); - - private: - ContextSwitcher(Isolate* isolate, int every_n_ms); - - Isolate* isolate() const { return isolate_; } - - void Run(); - - bool keep_going_; - int sleep_ms_; - Isolate* isolate_; -}; - } } // namespace v8::internal #endif // V8_V8THREADS_H_ diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 2df0a89..761b402 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -14478,116 +14478,6 @@ THREADED_TEST(CrossContextNew) { } -class ApplyInterruptTest { - public: - ApplyInterruptTest() : block_(0) {} - ~ApplyInterruptTest() {} - void RunTest() { - gc_count_ = 0; - gc_during_apply_ = 0; - apply_success_ = false; - gc_success_ = false; - GCThread gc_thread(this); - gc_thread.Start(); - v8::Isolate* isolate = CcTest::isolate(); - v8::Locker::StartPreemption(isolate, 1); - - LongRunningApply(); - { - v8::Unlocker unlock(isolate); - gc_thread.Join(); - } - v8::Locker::StopPreemption(isolate); - CHECK(apply_success_); - CHECK(gc_success_); - } - - private: - // Number of garbage collections required. - static const int kRequiredGCs = 2; - - class GCThread : public i::Thread { - public: - explicit GCThread(ApplyInterruptTest* test) - : Thread("GCThread"), test_(test) {} - virtual void Run() { - test_->CollectGarbage(); - } - private: - ApplyInterruptTest* test_; - }; - - void CollectGarbage() { - block_.Wait(); - while (gc_during_apply_ < kRequiredGCs) { - { - v8::Locker lock(CcTest::isolate()); - v8::Isolate::Scope isolate_scope(CcTest::isolate()); - CcTest::heap()->CollectAllGarbage(i::Heap::kNoGCFlags); - gc_count_++; - } - i::OS::Sleep(1); - } - gc_success_ = true; - } - - void LongRunningApply() { - block_.Signal(); - int rounds = 0; - while (gc_during_apply_ < kRequiredGCs) { - int gc_before = gc_count_; - { - const char* c_source = - "function do_very_little(bar) {" - " this.foo = bar;" - "}" - "for (var i = 0; i < 100000; i++) {" - " do_very_little.apply(this, ['bar']);" - "}"; - Local source = String::New(c_source); - Local