From 9bbf7887504b0fad38db21be54879bae81af8cf5 Mon Sep 17 00:00:00 2001 From: "jochen@chromium.org" Date: Wed, 8 Oct 2014 11:51:57 +0000 Subject: [PATCH] Get rid of isolate state. it's not thread safe, and there are only initialized isolates now. BUG=none R=svenpanne@chromium.org LOG=n Review URL: https://codereview.chromium.org/633363002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24466 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 22 +------ src/heap/heap.cc | 6 +- src/heap/heap.h | 8 +++ src/heap/incremental-marking.cc | 3 +- src/isolate.cc | 107 ++++++++++++++------------------ src/isolate.h | 18 +----- src/sampler.cc | 5 +- src/v8threads.cc | 9 --- 8 files changed, 65 insertions(+), 113 deletions(-) diff --git a/src/api.cc b/src/api.cc index 512eaeb347..4fbbc80708 100644 --- a/src/api.cc +++ b/src/api.cc @@ -53,7 +53,6 @@ #define LOG_API(isolate, expr) LOG(isolate, ApiEntryCall(expr)) #define ENTER_V8(isolate) \ - DCHECK((isolate)->IsInitialized()); \ i::VMState __state__((isolate)) namespace v8 { @@ -194,7 +193,6 @@ bool V8::IsDead() { static inline bool IsExecutionTerminatingCheck(i::Isolate* isolate) { - if (!isolate->IsInitialized()) return false; if (isolate->has_scheduled_exception()) { return isolate->scheduled_exception() == isolate->heap()->termination_exception(); @@ -2690,7 +2688,6 @@ Local Value::ToInteger() const { void i::Internals::CheckInitializedImpl(v8::Isolate* external_isolate) { i::Isolate* isolate = reinterpret_cast(external_isolate); Utils::ApiCheck(isolate != NULL && - isolate->IsInitialized() && !isolate->IsDead(), "v8::internal::Internals::CheckInitialized()", "Isolate is not initialized or V8 has died"); @@ -5736,7 +5733,6 @@ double v8::Date::ValueOf() const { void v8::Date::DateTimeConfigurationChangeNotification(Isolate* isolate) { i::Isolate* i_isolate = reinterpret_cast(isolate); - if (!i_isolate->IsInitialized()) return; ON_BAILOUT(i_isolate, "v8::Date::DateTimeConfigurationChangeNotification()", return); LOG_API(i_isolate, "Date::DateTimeConfigurationChangeNotification"); @@ -6284,7 +6280,6 @@ Local v8::Private::ForApi(Isolate* isolate, Local name) { Local v8::Number::New(Isolate* isolate, double value) { i::Isolate* internal_isolate = reinterpret_cast(isolate); - DCHECK(internal_isolate->IsInitialized()); if (std::isnan(value)) { // Introduce only canonical NaN value into the VM, to avoid signaling NaNs. value = base::OS::nan_value(); @@ -6297,7 +6292,6 @@ Local v8::Number::New(Isolate* isolate, double value) { Local v8::Integer::New(Isolate* isolate, int32_t value) { i::Isolate* internal_isolate = reinterpret_cast(isolate); - DCHECK(internal_isolate->IsInitialized()); if (i::Smi::IsValid(value)) { return Utils::IntegerToLocal(i::Handle(i::Smi::FromInt(value), internal_isolate)); @@ -6310,7 +6304,6 @@ Local v8::Integer::New(Isolate* isolate, int32_t value) { Local v8::Integer::NewFromUnsigned(Isolate* isolate, uint32_t value) { i::Isolate* internal_isolate = reinterpret_cast(isolate); - DCHECK(internal_isolate->IsInitialized()); bool fits_into_int32_t = (value & (1 << 31)) == 0; if (fits_into_int32_t) { return Integer::New(isolate, static_cast(value)); @@ -6598,7 +6591,7 @@ Isolate* Isolate::GetCurrent() { Isolate* Isolate::New(const Isolate::CreateParams& params) { - i::Isolate* isolate = new i::Isolate(); + i::Isolate* isolate = new i::Isolate(params.enable_serializer); Isolate* v8_isolate = reinterpret_cast(isolate); if (params.entry_hook) { isolate->set_function_entry_hook(params.entry_hook); @@ -6609,9 +6602,6 @@ Isolate* Isolate::New(const Isolate::CreateParams& params) { params.code_event_handler); } SetResourceConstraints(isolate, params.constraints); - if (params.enable_serializer) { - isolate->enable_serializer(); - } // TODO(jochen): Once we got rid of Isolate::Current(), we can remove this. Isolate::Scope isolate_scope(v8_isolate); if (params.entry_hook || !i::Snapshot::Initialize(isolate)) { @@ -6701,14 +6691,6 @@ Isolate::SuppressMicrotaskExecutionScope::~SuppressMicrotaskExecutionScope() { void Isolate::GetHeapStatistics(HeapStatistics* heap_statistics) { i::Isolate* isolate = reinterpret_cast(this); - if (!isolate->IsInitialized()) { - heap_statistics->total_heap_size_ = 0; - heap_statistics->total_heap_size_executable_ = 0; - heap_statistics->total_physical_size_ = 0; - heap_statistics->used_heap_size_ = 0; - heap_statistics->heap_size_limit_ = 0; - return; - } i::Heap* heap = isolate->heap(); heap_statistics->total_heap_size_ = heap->CommittedMemory(); heap_statistics->total_heap_size_executable_ = @@ -7004,7 +6986,6 @@ void Debug::SendCommand(Isolate* isolate, Local Debug::Call(v8::Handle fun, v8::Handle data) { i::Isolate* isolate = i::Isolate::Current(); - if (!isolate->IsInitialized()) return Local(); ON_BAILOUT(isolate, "v8::Debug::Call()", return Local()); ENTER_V8(isolate); i::MaybeHandle maybe_result; @@ -7025,7 +7006,6 @@ Local Debug::Call(v8::Handle fun, Local Debug::GetMirror(v8::Handle obj) { i::Isolate* isolate = i::Isolate::Current(); - if (!isolate->IsInitialized()) return Local(); ON_BAILOUT(isolate, "v8::Debug::GetMirror()", return Local()); ENTER_V8(isolate); v8::EscapableHandleScope scope(reinterpret_cast(isolate)); diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 0ccd0ab2f2..e164a398eb 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -133,7 +133,8 @@ Heap::Heap() configured_(false), external_string_table_(this), chunks_queued_for_free_(NULL), - gc_callbacks_depth_(0) { + gc_callbacks_depth_(0), + deserialization_complete_(false) { // Allow build-time customization of the max semispace size. Building // V8 with snapshots and a non-default max semispace size is much // easier if you can define it as part of the build environment. @@ -5188,6 +5189,9 @@ void Heap::SetStackLimits() { } +void Heap::NotifyDeserializationComplete() { deserialization_complete_ = true; } + + void Heap::TearDown() { #ifdef VERIFY_HEAP if (FLAG_verify_heap) { diff --git a/src/heap/heap.h b/src/heap/heap.h index c37464c9bc..f26fd77ff8 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -543,6 +543,10 @@ class Heap { // jslimit_/real_jslimit_ variable in the StackGuard. void SetStackLimits(); + // Notifies the heap that is ok to start marking or other activities that + // should not happen during deserialization. + void NotifyDeserializationComplete(); + // Returns whether SetUp has been called. bool HasBeenSetUp(); @@ -1378,6 +1382,8 @@ class Heap { inline void OnMoveEvent(HeapObject* target, HeapObject* source, int size_in_bytes); + bool deserialization_complete() const { return deserialization_complete_; } + protected: // Methods made available to tests. @@ -2034,6 +2040,8 @@ class Heap { int gc_callbacks_depth_; + bool deserialization_complete_; + friend class AlwaysAllocateScope; friend class Deserializer; friend class Factory; diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc index d72423a60a..8a1c25b709 100644 --- a/src/heap/incremental-marking.cc +++ b/src/heap/incremental-marking.cc @@ -439,8 +439,8 @@ bool IncrementalMarking::WorthActivating() { // 3) when we are currently not serializing or deserializing the heap. return FLAG_incremental_marking && FLAG_incremental_marking_steps && heap_->gc_state() == Heap::NOT_IN_GC && + heap_->deserialization_complete() && !heap_->isolate()->serializer_enabled() && - heap_->isolate()->IsInitialized() && heap_->PromotedSpaceSizeOfObjects() > kActivationThreshold; } @@ -516,7 +516,6 @@ void IncrementalMarking::Start(CompactionFlag flag) { DCHECK(state_ == STOPPED); DCHECK(heap_->gc_state() == Heap::NOT_IN_GC); DCHECK(!heap_->isolate()->serializer_enabled()); - DCHECK(heap_->isolate()->IsInitialized()); ResetStepCounters(); diff --git a/src/isolate.cc b/src/isolate.cc index a5ee021b14..e8e1d66602 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -584,13 +584,6 @@ static void PrintFrames(Isolate* isolate, void Isolate::PrintStack(StringStream* accumulator) { - if (!IsInitialized()) { - accumulator->Add( - "\n==== JS stack trace is not available =======================\n\n"); - accumulator->Add( - "\n==== Isolate for the thread is not initialized =============\n\n"); - return; - } // The MentionedObjectCache is not GC-proof at the moment. DisallowHeapAllocation no_gc; DCHECK(StringStream::IsMentionedObjectCacheClear(this)); @@ -1467,9 +1460,8 @@ void Isolate::ThreadDataTable::RemoveAllThreads(Isolate* isolate) { #endif -Isolate::Isolate() +Isolate::Isolate(bool enable_serializer) : embedder_data_(), - state_(UNINITIALIZED), entry_stack_(NULL), stack_trace_nesting_level_(0), incomplete_message_(NULL), @@ -1507,7 +1499,7 @@ Isolate::Isolate() // TODO(bmeurer) Initialized lazily because it depends on flags; can // be fixed once the default isolate cleanup is done. random_number_generator_(NULL), - serializer_enabled_(false), + serializer_enabled_(enable_serializer), has_fatal_error_(false), initialized_from_snapshot_(false), cpu_profiler_(NULL), @@ -1596,58 +1588,53 @@ void Isolate::GlobalTearDown() { void Isolate::Deinit() { - if (state_ == INITIALIZED) { - TRACE_ISOLATE(deinit); + TRACE_ISOLATE(deinit); - debug()->Unload(); + debug()->Unload(); - FreeThreadResources(); + FreeThreadResources(); - if (concurrent_recompilation_enabled()) { - optimizing_compiler_thread_->Stop(); - delete optimizing_compiler_thread_; - optimizing_compiler_thread_ = NULL; - } - - if (heap_.mark_compact_collector()->sweeping_in_progress()) { - heap_.mark_compact_collector()->EnsureSweepingCompleted(); - } - - if (FLAG_turbo_stats) GetTStatistics()->Print("TurboFan"); - if (FLAG_hydrogen_stats) GetHStatistics()->Print("Hydrogen"); - - if (FLAG_print_deopt_stress) { - PrintF(stdout, "=== Stress deopt counter: %u\n", stress_deopt_count_); - } - - // We must stop the logger before we tear down other components. - Sampler* sampler = logger_->sampler(); - if (sampler && sampler->IsActive()) sampler->Stop(); - - delete deoptimizer_data_; - deoptimizer_data_ = NULL; - builtins_.TearDown(); - bootstrapper_->TearDown(); - - if (runtime_profiler_ != NULL) { - delete runtime_profiler_; - runtime_profiler_ = NULL; - } - - delete basic_block_profiler_; - basic_block_profiler_ = NULL; - - heap_.TearDown(); - logger_->TearDown(); - - delete heap_profiler_; - heap_profiler_ = NULL; - delete cpu_profiler_; - cpu_profiler_ = NULL; - - // The default isolate is re-initializable due to legacy API. - state_ = UNINITIALIZED; + if (concurrent_recompilation_enabled()) { + optimizing_compiler_thread_->Stop(); + delete optimizing_compiler_thread_; + optimizing_compiler_thread_ = NULL; } + + if (heap_.mark_compact_collector()->sweeping_in_progress()) { + heap_.mark_compact_collector()->EnsureSweepingCompleted(); + } + + if (FLAG_turbo_stats) GetTStatistics()->Print("TurboFan"); + if (FLAG_hydrogen_stats) GetHStatistics()->Print("Hydrogen"); + + if (FLAG_print_deopt_stress) { + PrintF(stdout, "=== Stress deopt counter: %u\n", stress_deopt_count_); + } + + // We must stop the logger before we tear down other components. + Sampler* sampler = logger_->sampler(); + if (sampler && sampler->IsActive()) sampler->Stop(); + + delete deoptimizer_data_; + deoptimizer_data_ = NULL; + builtins_.TearDown(); + bootstrapper_->TearDown(); + + if (runtime_profiler_ != NULL) { + delete runtime_profiler_; + runtime_profiler_ = NULL; + } + + delete basic_block_profiler_; + basic_block_profiler_ = NULL; + + heap_.TearDown(); + logger_->TearDown(); + + delete heap_profiler_; + heap_profiler_ = NULL; + delete cpu_profiler_; + cpu_profiler_ = NULL; } @@ -1824,7 +1811,6 @@ void Isolate::InitializeLoggingAndCounters() { bool Isolate::Init(Deserializer* des) { - DCHECK(state_ != INITIALIZED); TRACE_ISOLATE(init); stress_deopt_count_ = FLAG_deopt_every_n_times; @@ -1990,9 +1976,10 @@ bool Isolate::Init(Deserializer* des) { heap_.amount_of_external_allocated_memory_at_last_global_gc_)), Internals::kAmountOfExternalAllocatedMemoryAtLastGlobalGCOffset); - state_ = INITIALIZED; time_millis_at_init_ = base::OS::TimeCurrentMillis(); + heap_.NotifyDeserializationComplete(); + if (!create_heap_objects) { // Now that the heap is consistent, it's OK to generate the code for the // deopt entry table that might have been referred to by optimized code in diff --git a/src/isolate.h b/src/isolate.h index a94678d135..f1e6a3e326 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -511,8 +511,6 @@ class Isolate { bool Init(Deserializer* des); - bool IsInitialized() { return state_ == INITIALIZED; } - // True if at least one thread Enter'ed this isolate. bool IsInUse() { return entry_stack_ != NULL; } @@ -1004,12 +1002,6 @@ class Isolate { THREAD_LOCAL_TOP_ACCESSOR(LookupResult*, top_lookup_result) - void enable_serializer() { - // The serializer can only be enabled before the isolate init. - DCHECK(state_ != INITIALIZED); - serializer_enabled_ = true; - } - bool serializer_enabled() const { return serializer_enabled_; } bool IsDead() { return has_fatal_error_; } @@ -1113,25 +1105,19 @@ class Isolate { BasicBlockProfiler* GetOrCreateBasicBlockProfiler(); BasicBlockProfiler* basic_block_profiler() { return basic_block_profiler_; } - static Isolate* NewForTesting() { return new Isolate(); } + static Isolate* NewForTesting() { return new Isolate(false); } private: - Isolate(); + explicit Isolate(bool enable_serializer); friend struct GlobalState; friend struct InitializeGlobalState; - enum State { - UNINITIALIZED, // Some components may not have been allocated. - INITIALIZED // All components are fully initialized. - }; - // These fields are accessed through the API, offsets must be kept in sync // with v8::internal::Internals (in include/v8.h) constants. This is also // verified in Isolate::Init() using runtime checks. void* embedder_data_[Internals::kNumIsolateDataSlots]; Heap heap_; - State state_; // Will be padded to kApiPointerSize. // The per-process lock should be acquired before the ThreadDataTable is // modified. diff --git a/src/sampler.cc b/src/sampler.cc index 4633712f1c..95b7f4bc33 100644 --- a/src/sampler.cc +++ b/src/sampler.cc @@ -335,7 +335,7 @@ void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info, USE(info); if (signal != SIGPROF) return; Isolate* isolate = Isolate::UnsafeCurrent(); - if (isolate == NULL || !isolate->IsInitialized() || !isolate->IsInUse()) { + if (isolate == NULL || !isolate->IsInUse()) { // We require a fully initialized and entered isolate. return; } @@ -542,7 +542,6 @@ class SamplerThread : public base::Thread { // profiled. We must not suspend. for (int i = 0; i < active_samplers_.length(); ++i) { Sampler* sampler = active_samplers_.at(i); - if (!sampler->isolate()->IsInitialized()) continue; if (!sampler->IsProfiling()) continue; sampler->DoSample(); } @@ -572,7 +571,6 @@ SamplerThread* SamplerThread::instance_ = NULL; // DISABLE_ASAN void TickSample::Init(Isolate* isolate, const v8::RegisterState& regs) { - DCHECK(isolate->IsInitialized()); timestamp = base::TimeTicks::HighResolutionNow(); pc = reinterpret_cast
(regs.pc); state = isolate->current_vm_state(); @@ -612,7 +610,6 @@ DISABLE_ASAN void TickSample::Init(Isolate* isolate, void TickSample::GetStackSample(Isolate* isolate, const v8::RegisterState& regs, void** frames, size_t frames_limit, v8::SampleInfo* sample_info) { - DCHECK(isolate->IsInitialized()); sample_info->frames_count = 0; sample_info->vm_state = isolate->current_vm_state(); if (sample_info->vm_state == GC) return; diff --git a/src/v8threads.cc b/src/v8threads.cc index 788786dbfd..b799126563 100644 --- a/src/v8threads.cc +++ b/src/v8threads.cc @@ -37,15 +37,6 @@ void Locker::Initialize(v8::Isolate* isolate) { isolate_->thread_manager()->Lock(); has_lock_ = true; - // Make sure that V8 is initialized. Archiving of threads interferes - // with deserialization by adding additional root pointers, so we must - // initialize here, before anyone can call ~Locker() or Unlocker(). - if (!isolate_->IsInitialized()) { - isolate_->Enter(); - V8::Initialize(); - isolate_->Exit(); - } - // This may be a locker within an unlocker in which case we have to // get the saved state for this thread and restore it. if (isolate_->thread_manager()->RestoreThread()) {