From 46b3582f48a3026b3087f4548b7cb3b2ff5b1102 Mon Sep 17 00:00:00 2001 From: yangguo Date: Tue, 28 Apr 2015 04:13:22 -0700 Subject: [PATCH] Reland: Preprocess structured stack trace on GC to get rid of code reference. BUG=v8:2340 LOG=N Review URL: https://codereview.chromium.org/1109093002 Cr-Commit-Position: refs/heads/master@{#28102} --- src/factory.h | 4 ++++ src/heap/heap.cc | 31 +++++++++++++++++++++++++++++++ src/heap/heap.h | 5 ++++- src/isolate.cc | 6 ++++++ src/messages.js | 2 +- src/objects-inl.h | 2 +- src/objects.cc | 2 +- src/objects.h | 3 ++- test/cctest/test-heap.cc | 33 +++++++++++++++++++++++++++++++++ 9 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/factory.h b/src/factory.h index 72991d9..0ab312b 100644 --- a/src/factory.h +++ b/src/factory.h @@ -644,6 +644,10 @@ class Factory final { isolate()->heap()->set_string_table(*table); } + inline void set_weak_stack_trace_list(Handle list) { + isolate()->heap()->set_weak_stack_trace_list(*list); + } + Handle hidden_string() { return Handle(&isolate()->heap()->hidden_string_); } diff --git a/src/heap/heap.cc b/src/heap/heap.cc index e9379ac..4907030 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -706,6 +706,32 @@ void Heap::GarbageCollectionEpilogue() { } +void Heap::PreprocessStackTraces() { + if (!weak_stack_trace_list()->IsWeakFixedArray()) return; + WeakFixedArray* array = WeakFixedArray::cast(weak_stack_trace_list()); + int length = array->Length(); + for (int i = 0; i < length; i++) { + if (array->IsEmptySlot(i)) continue; + FixedArray* elements = FixedArray::cast(array->Get(i)); + for (int j = 1; j < elements->length(); j += 4) { + Object* maybe_code = elements->get(j + 2); + // If GC happens while adding a stack trace to the weak fixed array, + // which has been copied into a larger backing store, we may run into + // a stack trace that has already been preprocessed. Guard against this. + if (!maybe_code->IsCode()) break; + Code* code = Code::cast(maybe_code); + int offset = Smi::cast(elements->get(j + 3))->value(); + Address pc = code->address() + offset; + int pos = code->SourcePosition(pc); + elements->set(j + 2, Smi::FromInt(pos)); + } + } + // We must not compact the weak fixed list here, as we may be in the middle + // of writing to it, when the GC triggered. Instead, we reset the root value. + set_weak_stack_trace_list(Smi::FromInt(0)); +} + + void Heap::HandleGCRequest() { if (incremental_marking()->request_type() == IncrementalMarking::COMPLETE_MARKING) { @@ -1272,6 +1298,8 @@ void Heap::MarkCompactEpilogue() { isolate_->counters()->objs_since_last_full()->Set(0); incremental_marking()->Epilogue(); + + PreprocessStackTraces(); } @@ -3082,6 +3110,8 @@ void Heap::CreateInitialObjects() { cell->set_value(Smi::FromInt(Isolate::kArrayProtectorValid)); set_array_protector(*cell); + set_weak_stack_trace_list(Smi::FromInt(0)); + set_allocation_sites_scratchpad( *factory->NewFixedArray(kAllocationSiteScratchpadSize, TENURED)); InitializeAllocationSitesScratchpad(); @@ -3118,6 +3148,7 @@ bool Heap::RootCanBeWrittenAfterInitialization(Heap::RootListIndex root_index) { case kDetachedContextsRootIndex: case kWeakObjectToCodeTableRootIndex: case kRetainedMapsRootIndex: + case kWeakStackTraceListRootIndex: // Smi values #define SMI_ENTRY(type, name, Name) case k##Name##RootIndex: SMI_ROOT_LIST(SMI_ENTRY) diff --git a/src/heap/heap.h b/src/heap/heap.h index 06f9304..2c8a6bc 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -186,7 +186,8 @@ namespace internal { V(FixedArray, detached_contexts, DetachedContexts) \ V(ArrayList, retained_maps, RetainedMaps) \ V(WeakHashTable, weak_object_to_code_table, WeakObjectToCodeTable) \ - V(PropertyCell, array_protector, ArrayProtector) + V(PropertyCell, array_protector, ArrayProtector) \ + V(Object, weak_stack_trace_list, WeakStackTraceList) // Entries in this list are limited to Smis and are not visited during GC. #define SMI_ROOT_LIST(V) \ @@ -1742,6 +1743,8 @@ class Heap { void GarbageCollectionPrologue(); void GarbageCollectionEpilogue(); + void PreprocessStackTraces(); + // Pretenuring decisions are made based on feedback collected during new // space evacuation. Note that between feedback collection and calling this // method object in old space must not move. diff --git a/src/isolate.cc b/src/isolate.cc index b526a01..ff94105 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -407,8 +407,14 @@ Handle Isolate::CaptureSimpleStackTrace(Handle error_object, } } elements->set(0, Smi::FromInt(sloppy_frames)); + elements->Shrink(cursor); Handle result = factory()->NewJSArrayWithElements(elements); result->set_length(Smi::FromInt(cursor)); + // Queue this structured stack trace for preprocessing on GC. + Handle old_weak_list(heap()->weak_stack_trace_list(), this); + Handle new_weak_list = + WeakFixedArray::Add(old_weak_list, elements); + factory()->set_weak_stack_trace_list(new_weak_list); return result; } diff --git a/src/messages.js b/src/messages.js index 9ad3840..f7469ee 100644 --- a/src/messages.js +++ b/src/messages.js @@ -998,7 +998,7 @@ function GetStackFrames(raw_stack) { var fun = raw_stack[i + 1]; var code = raw_stack[i + 2]; var pc = raw_stack[i + 3]; - var pos = %FunctionGetPositionForOffset(code, pc); + var pos = %_IsSmi(code) ? code : %FunctionGetPositionForOffset(code, pc); sloppy_frames--; frames.push(new CallSite(recv, fun, pos, (sloppy_frames < 0))); } diff --git a/src/objects-inl.h b/src/objects-inl.h index 647b123..e3dc749 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -2356,7 +2356,7 @@ bool WeakFixedArray::IsEmptySlot(int index) const { } -void WeakFixedArray::clear(int index) { +void WeakFixedArray::Clear(int index) { FixedArray::cast(this)->set(index + kFirstIndex, Smi::FromInt(0)); } diff --git a/src/objects.cc b/src/objects.cc index 7e3c70a..9b0e8a4 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -8348,7 +8348,7 @@ bool WeakFixedArray::Remove(Handle value) { int first_index = last_used_index(); for (int i = first_index;;) { if (Get(i) == *value) { - clear(i); + Clear(i); // Users of WeakFixedArray should make sure that there are no duplicates, // they can use Add(..., kAddIfNotFound) if necessary. return true; diff --git a/src/objects.h b/src/objects.h index 1a6c179..0b3c526 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2632,8 +2632,10 @@ class WeakFixedArray : public FixedArray { void Compact(); inline Object* Get(int index) const; + inline void Clear(int index); inline int Length() const; + inline bool IsEmptySlot(int index) const; static Object* Empty() { return Smi::FromInt(0); } DECLARE_CAST(WeakFixedArray) @@ -2648,7 +2650,6 @@ class WeakFixedArray : public FixedArray { static void Set(Handle array, int index, Handle value); inline void clear(int index); - inline bool IsEmptySlot(int index) const; inline int last_used_index() const; inline void set_last_used_index(int index); diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index b8d7676..906b4fe 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -5379,3 +5379,36 @@ TEST(WeakFixedArray) { array->Compact(); WeakFixedArray::Add(array, number); } + + +TEST(PreprocessStackTrace) { + // Do not automatically trigger early GC. + FLAG_gc_interval = -1; + CcTest::InitializeVM(); + v8::HandleScope scope(CcTest::isolate()); + v8::TryCatch try_catch; + CompileRun("throw new Error();"); + CHECK(try_catch.HasCaught()); + Isolate* isolate = CcTest::i_isolate(); + Handle exception = v8::Utils::OpenHandle(*try_catch.Exception()); + Handle key = isolate->factory()->stack_trace_symbol(); + Handle stack_trace = + JSObject::GetProperty(exception, key).ToHandleChecked(); + Handle code = + Object::GetElement(isolate, stack_trace, 3).ToHandleChecked(); + CHECK(code->IsCode()); + + isolate->heap()->CollectAllAvailableGarbage("stack trace preprocessing"); + + Handle pos = + Object::GetElement(isolate, stack_trace, 3).ToHandleChecked(); + CHECK(pos->IsSmi()); + + Handle stack_trace_array = Handle::cast(stack_trace); + int array_length = Smi::cast(stack_trace_array->length())->value(); + for (int i = 0; i < array_length; i++) { + Handle element = + Object::GetElement(isolate, stack_trace, i).ToHandleChecked(); + CHECK(!element->IsCode()); + } +} -- 2.7.4