Revision: 25083
Author: [email protected]
Date: Mon Nov 3 17:23:55 2014 UTC
Log: Introduce phantom weak handles in the API and use them internally
for debug info
[email protected], [email protected]
BUG=
Review URL: https://codereview.chromium.org/687003005
https://code.google.com/p/v8/source/detail?r=25083
Modified:
/branches/bleeding_edge/include/v8.h
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/compiler.cc
/branches/bleeding_edge/src/debug.cc
/branches/bleeding_edge/src/debug.h
/branches/bleeding_edge/src/global-handles.cc
/branches/bleeding_edge/src/global-handles.h
/branches/bleeding_edge/src/globals.h
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/include/v8.h Thu Oct 30 14:51:17 2014 UTC
+++ /branches/bleeding_edge/include/v8.h Mon Nov 3 17:23:55 2014 UTC
@@ -516,6 +516,18 @@
P* parameter,
typename WeakCallbackData<S, P>::Callback callback);
+ // Phantom persistents work like weak persistents, except that the
pointer to
+ // the object being collected is not available in the finalization
callback.
+ // This enables the garbage collector to collect the object and any
objects
+ // it references transitively in one GC cycle.
+ template <typename P>
+ V8_INLINE void SetPhantom(P* parameter,
+ typename WeakCallbackData<T, P>::Callback
callback);
+
+ template <typename S, typename P>
+ V8_INLINE void SetPhantom(P* parameter,
+ typename WeakCallbackData<S, P>::Callback
callback);
+
template<typename P>
V8_INLINE P* ClearWeak();
@@ -5477,14 +5489,15 @@
private:
V8();
+ enum WeakHandleType { PhantomHandle, NonphantomHandle };
+
static internal::Object** GlobalizeReference(internal::Isolate* isolate,
internal::Object** handle);
static internal::Object** CopyPersistent(internal::Object** handle);
static void DisposeGlobal(internal::Object** global_handle);
typedef WeakCallbackData<Value, void>::Callback WeakCallback;
- static void MakeWeak(internal::Object** global_handle,
- void* data,
- WeakCallback weak_callback);
+ static void MakeWeak(internal::Object** global_handle, void* data,
+ WeakCallback weak_callback, WeakHandleType phantom);
static void* ClearWeak(internal::Object** global_handle);
static void Eternalize(Isolate* isolate,
Value* handle,
@@ -6355,9 +6368,8 @@
typename WeakCallbackData<S, P>::Callback callback) {
TYPE_CHECK(S, T);
typedef typename WeakCallbackData<Value, void>::Callback Callback;
- V8::MakeWeak(reinterpret_cast<internal::Object**>(this->val_),
- parameter,
- reinterpret_cast<Callback>(callback));
+ V8::MakeWeak(reinterpret_cast<internal::Object**>(this->val_), parameter,
+ reinterpret_cast<Callback>(callback), V8::NonphantomHandle);
}
@@ -6371,7 +6383,26 @@
template <class T>
-template<typename P>
+template <typename S, typename P>
+void PersistentBase<T>::SetPhantom(
+ P* parameter, typename WeakCallbackData<S, P>::Callback callback) {
+ TYPE_CHECK(S, T);
+ typedef typename WeakCallbackData<Value, void>::Callback Callback;
+ V8::MakeWeak(reinterpret_cast<internal::Object**>(this->val_), parameter,
+ reinterpret_cast<Callback>(callback), V8::PhantomHandle);
+}
+
+
+template <class T>
+template <typename P>
+void PersistentBase<T>::SetPhantom(
+ P* parameter, typename WeakCallbackData<T, P>::Callback callback) {
+ SetPhantom<T, P>(parameter, callback);
+}
+
+
+template <class T>
+template <typename P>
P* PersistentBase<T>::ClearWeak() {
return reinterpret_cast<P*>(
V8::ClearWeak(reinterpret_cast<internal::Object**>(this->val_)));
=======================================
--- /branches/bleeding_edge/src/api.cc Thu Oct 30 14:51:17 2014 UTC
+++ /branches/bleeding_edge/src/api.cc Mon Nov 3 17:23:55 2014 UTC
@@ -490,10 +490,12 @@
}
-void V8::MakeWeak(i::Object** object,
- void* parameters,
- WeakCallback weak_callback) {
- i::GlobalHandles::MakeWeak(object, parameters, weak_callback);
+void V8::MakeWeak(i::Object** object, void* parameters,
+ WeakCallback weak_callback, V8::WeakHandleType
weak_type) {
+ i::GlobalHandles::PhantomState phantom;
+ phantom = weak_type == V8::PhantomHandle ? i::GlobalHandles::Phantom
+ : i::GlobalHandles::Nonphantom;
+ i::GlobalHandles::MakeWeak(object, parameters, weak_callback, phantom);
}
=======================================
--- /branches/bleeding_edge/src/compiler.cc Thu Oct 30 14:40:30 2014 UTC
+++ /branches/bleeding_edge/src/compiler.cc Mon Nov 3 17:23:55 2014 UTC
@@ -1308,7 +1308,7 @@
if (outer_info->is_toplevel() && outer_info->will_serialize()) {
// Make sure that if the toplevel code (possibly to be serialized),
- // the inner unction must be allowed to be compiled lazily.
+ // the inner function must be allowed to be compiled lazily.
DCHECK(allow_lazy);
}
=======================================
--- /branches/bleeding_edge/src/debug.cc Wed Oct 29 10:19:08 2014 UTC
+++ /branches/bleeding_edge/src/debug.cc Mon Nov 3 17:23:55 2014 UTC
@@ -594,11 +594,8 @@
Heap* heap = isolate_->heap();
HandleScope scope(isolate_);
- // Perform two GCs to get rid of all unreferenced scripts. The first GC
gets
- // rid of all the cached script wrappers and the second gets rid of the
- // scripts which are no longer referenced.
+ // Perform a GC to get rid of all unreferenced scripts.
heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "ScriptCache");
- heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "ScriptCache");
// Scan heap for Script objects.
HeapIterator iterator(heap);
@@ -694,13 +691,7 @@
Debug* debug = reinterpret_cast<Isolate*>(data.GetIsolate())->debug();
DebugInfoListNode* node =
reinterpret_cast<DebugInfoListNode*>(data.GetParameter());
- // We need to clear all breakpoints associated with the function to
restore
- // original code and avoid patching the code twice later because
- // the function will live in the heap until next gc, and can be found by
- // Debug::FindSharedFunctionInfoInScript.
- BreakLocationIterator it(node->debug_info(), ALL_BREAK_LOCATIONS);
- it.ClearAllDebugBreak();
- debug->RemoveDebugInfo(node->debug_info());
+ debug->RemoveDebugInfo(node->debug_info().location());
#ifdef DEBUG
for (DebugInfoListNode* n = debug->debug_info_list_;
n != NULL;
@@ -716,8 +707,8 @@
GlobalHandles* global_handles =
debug_info->GetIsolate()->global_handles();
debug_info_ =
Handle<DebugInfo>::cast(global_handles->Create(debug_info));
GlobalHandles::MakeWeak(reinterpret_cast<Object**>(debug_info_.location()),
- this,
- Debug::HandleWeakDebugInfo);
+ this, Debug::HandleWeakDebugInfo,
+ GlobalHandles::Phantom);
}
@@ -1172,7 +1163,7 @@
// If there are no more break points left remove the debug info for
this
// function.
if (debug_info->GetBreakPointCount() == 0) {
- RemoveDebugInfo(debug_info);
+ RemoveDebugInfoAndClearFromShared(debug_info);
}
return;
@@ -1193,7 +1184,7 @@
// Remove all debug info.
while (debug_info_list_ != NULL) {
- RemoveDebugInfo(debug_info_list_->debug_info());
+ RemoveDebugInfoAndClearFromShared(debug_info_list_->debug_info());
}
}
@@ -2211,21 +2202,23 @@
}
-void Debug::RemoveDebugInfo(Handle<DebugInfo> debug_info) {
+// This uses the location of a handle to look up the debug info in the
debug
+// info list, but it doesn't use the actual debug info for anything.
Therefore
+// if the debug info has been collected by the GC, we can be sure that this
+// method will not attempt to resurrect it.
+void Debug::RemoveDebugInfo(DebugInfo** debug_info) {
DCHECK(debug_info_list_ != NULL);
// Run through the debug info objects to find this one and remove it.
DebugInfoListNode* prev = NULL;
DebugInfoListNode* current = debug_info_list_;
while (current != NULL) {
- if (*current->debug_info() == *debug_info) {
+ if (current->debug_info().location() == debug_info) {
// Unlink from list. If prev is NULL we are looking at the first
element.
if (prev == NULL) {
debug_info_list_ = current->next();
} else {
prev->set_next(current->next());
}
- current->debug_info()->shared()->set_debug_info(
- isolate_->heap()->undefined_value());
delete current;
// If there are no more debug info objects there are not more break
@@ -2240,6 +2233,16 @@
}
UNREACHABLE();
}
+
+
+void Debug::RemoveDebugInfoAndClearFromShared(Handle<DebugInfo>
debug_info) {
+ HandleScope scope(isolate_);
+ Handle<SharedFunctionInfo> shared(debug_info->shared());
+
+ RemoveDebugInfo(debug_info.location());
+
+ shared->set_debug_info(isolate_->heap()->undefined_value());
+}
void Debug::SetAfterBreakTarget(JavaScriptFrame* frame) {
@@ -2336,7 +2339,7 @@
HandleScope scope(isolate_);
// If there are no break points this cannot be break at return, as
- // the debugger statement and stack guard bebug break cannot be at
+ // the debugger statement and stack guard debug break cannot be at
// return.
if (!has_break_points_) {
return false;
@@ -3257,7 +3260,7 @@
v8::Handle<v8::Context> MessageImpl::GetEventContext() const {
Isolate* isolate = event_data_->GetIsolate();
v8::Handle<v8::Context> context = GetDebugEventContext(isolate);
- // Isolate::context() may be NULL when "script collected" event occures.
+ // Isolate::context() may be NULL when "script collected" event occurs.
DCHECK(!context.IsEmpty());
return context;
}
=======================================
--- /branches/bleeding_edge/src/debug.h Mon Oct 27 12:03:25 2014 UTC
+++ /branches/bleeding_edge/src/debug.h Mon Nov 3 17:23:55 2014 UTC
@@ -559,8 +559,8 @@
void ClearStepIn();
void ActivateStepOut(StackFrame* frame);
void ClearStepNext();
- // Returns whether the compile succeeded.
- void RemoveDebugInfo(Handle<DebugInfo> debug_info);
+ void RemoveDebugInfoAndClearFromShared(Handle<DebugInfo> debug_info);
+ void RemoveDebugInfo(DebugInfo** debug_info);
Handle<Object> CheckBreakPoints(Handle<Object> break_point);
bool CheckBreakPoint(Handle<Object> break_point_object);
=======================================
--- /branches/bleeding_edge/src/global-handles.cc Mon Oct 27 12:03:25 2014
UTC
+++ /branches/bleeding_edge/src/global-handles.cc Mon Nov 3 17:23:55 2014
UTC
@@ -145,6 +145,13 @@
void set_in_new_space_list(bool v) {
flags_ = IsInNewSpaceList::update(flags_, v);
}
+
+ bool is_zapped_during_weak_callback() {
+ return IsZappedDuringWeakCallback::decode(flags_);
+ }
+ void set_is_zapped_during_weak_callback(bool v) {
+ flags_ = IsZappedDuringWeakCallback::update(flags_, v);
+ }
bool IsNearDeath() const {
// Check for PENDING to ensure correct answer when processing
callbacks.
@@ -204,12 +211,14 @@
parameter_or_next_free_.next_free = value;
}
- void MakeWeak(void* parameter, WeakCallback weak_callback) {
+ void MakeWeak(void* parameter, WeakCallback weak_callback,
+ bool is_zapped_during_weak_callback = false) {
DCHECK(weak_callback != NULL);
DCHECK(state() != FREE);
CHECK(object_ != NULL);
set_state(WEAK);
set_parameter(parameter);
+ set_is_zapped_during_weak_callback(is_zapped_during_weak_callback);
weak_callback_ = weak_callback;
}
@@ -227,7 +236,7 @@
Release();
return false;
}
- void* par = parameter();
+ void* param = parameter();
set_state(NEAR_DEATH);
set_parameter(NULL);
@@ -240,14 +249,28 @@
DCHECK(!object_->IsExternalTwoByteString() ||
ExternalTwoByteString::cast(object_)->resource() != NULL);
// Leaving V8.
- VMState<EXTERNAL> state(isolate);
+ VMState<EXTERNAL> vmstate(isolate);
HandleScope handle_scope(isolate);
- Handle<Object> handle(*object, isolate);
- v8::WeakCallbackData<v8::Value, void> data(
- reinterpret_cast<v8::Isolate*>(isolate),
- v8::Utils::ToLocal(handle),
- par);
- weak_callback_(data);
+ if (is_zapped_during_weak_callback()) {
+ // Phantom weak pointer case.
+ DCHECK(*object == Smi::FromInt(kPhantomReferenceZap));
+ // Make data with a null handle.
+ v8::WeakCallbackData<v8::Value, void> data(
+ reinterpret_cast<v8::Isolate*>(isolate),
v8::Local<v8::Object>(),
+ param);
+ weak_callback_(data);
+ if (state() != FREE) {
+ // Callback does not have to clear the global handle if it is a
+ // phantom handle.
+ Release();
+ }
+ } else {
+ Handle<Object> handle(*object, isolate);
+ v8::WeakCallbackData<v8::Value, void> data(
+ reinterpret_cast<v8::Isolate*>(isolate),
v8::Utils::ToLocal(handle),
+ param);
+ weak_callback_(data);
+ }
}
// Absence of explicit cleanup or revival of weak handle
// in most of the cases would lead to memory leak.
@@ -277,10 +300,11 @@
// This stores three flags (independent, partially_dependent and
// in_new_space_list) and a State.
- class NodeState: public BitField<State, 0, 4> {};
- class IsIndependent: public BitField<bool, 4, 1> {};
- class IsPartiallyDependent: public BitField<bool, 5, 1> {};
- class IsInNewSpaceList: public BitField<bool, 6, 1> {};
+ class NodeState : public BitField<State, 0, 4> {};
+ class IsIndependent : public BitField<bool, 4, 1> {};
+ class IsPartiallyDependent : public BitField<bool, 5, 1> {};
+ class IsInNewSpaceList : public BitField<bool, 6, 1> {};
+ class IsZappedDuringWeakCallback : public BitField<bool, 7, 1> {};
uint8_t flags_;
@@ -475,10 +499,10 @@
}
-void GlobalHandles::MakeWeak(Object** location,
- void* parameter,
- WeakCallback weak_callback) {
- Node::FromLocation(location)->MakeWeak(parameter, weak_callback);
+void GlobalHandles::MakeWeak(Object** location, void* parameter,
+ WeakCallback weak_callback, PhantomState
phantom) {
+ Node::FromLocation(location)
+ ->MakeWeak(parameter, weak_callback, phantom == Phantom);
}
@@ -514,7 +538,15 @@
void GlobalHandles::IterateWeakRoots(ObjectVisitor* v) {
for (NodeIterator it(this); !it.done(); it.Advance()) {
- if (it.node()->IsWeakRetainer())
v->VisitPointer(it.node()->location());
+ Node* node = it.node();
+ if (node->IsWeakRetainer()) {
+ if (node->state() == Node::PENDING &&
+ node->is_zapped_during_weak_callback()) {
+ *(node->location()) = Smi::FromInt(kPhantomReferenceZap);
+ } else {
+ v->VisitPointer(node->location());
+ }
+ }
}
}
@@ -559,7 +591,11 @@
DCHECK(node->is_in_new_space_list());
if ((node->is_independent() || node->is_partially_dependent()) &&
node->IsWeakRetainer()) {
- v->VisitPointer(node->location());
+ if (node->is_zapped_during_weak_callback()) {
+ *(node->location()) = Smi::FromInt(kPhantomReferenceZap);
+ } else {
+ v->VisitPointer(node->location());
+ }
}
}
}
=======================================
--- /branches/bleeding_edge/src/global-handles.h Mon Oct 27 12:03:25 2014
UTC
+++ /branches/bleeding_edge/src/global-handles.h Mon Nov 3 17:23:55 2014
UTC
@@ -112,15 +112,24 @@
typedef WeakCallbackData<v8::Value, void>::Callback WeakCallback;
+ // For a phantom weak reference, the callback does not have access to the
+ // dying object. Phantom weak references are preferred because they
allow
+ // memory to be reclaimed in one GC cycle rather than two. However, for
+ // historical reasons the default is non-phantom.
+ enum PhantomState { Nonphantom, Phantom };
+
// Make the global handle weak and set the callback parameter for the
// handle. When the garbage collector recognizes that only weak global
- // handles point to an object the handles are cleared and the callback
- // function is invoked (for each handle) with the handle and
corresponding
- // parameter as arguments. Note: cleared means set to Smi::FromInt(0).
The
- // reason is that Smi::FromInt(0) does not change during garage
collection.
- static void MakeWeak(Object** location,
- void* parameter,
- WeakCallback weak_callback);
+ // handles point to an object the callback function is invoked (for each
+ // handle) with the handle and corresponding parameter as arguments. By
+ // default the handle still contains a pointer to the object that is
being
+ // collected. For this reason the object is not collected until the next
+ // GC. For a phantom weak handle the handle is cleared (set to a Smi)
+ // before the callback is invoked, but the handle can still be identified
+ // in the callback by using the location() of the handle.
+ static void MakeWeak(Object** location, void* parameter,
+ WeakCallback weak_callback,
+ PhantomState phantom = Nonphantom);
void RecordStats(HeapStats* stats);
=======================================
--- /branches/bleeding_edge/src/globals.h Mon Nov 3 08:43:20 2014 UTC
+++ /branches/bleeding_edge/src/globals.h Mon Nov 3 17:23:55 2014 UTC
@@ -288,6 +288,7 @@
#endif
const int kCodeZapValue = 0xbadc0de;
+const uint32_t kPhantomReferenceZap = 0xca11bac;
// On Intel architecture, cache line size is 64 bytes.
// On ARM it may be less (32 bytes), but as far this constant is
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Oct 30 14:51:17
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Mon Nov 3 17:23:55
2014 UTC
@@ -7487,14 +7487,13 @@
};
-static void DisposeAndSetFlag(
+static void SetFlag(
const v8::WeakCallbackData<v8::Object, FlagAndPersistent>& data) {
- data.GetParameter()->handle.Reset();
data.GetParameter()->flag = true;
}
-THREADED_TEST(IndependentWeakHandle) {
+static void IndependentWeakHandle(bool global_gc, bool interlinked) {
v8::Isolate* iso = CcTest::isolate();
v8::HandleScope scope(iso);
v8::Handle<Context> context = Context::New(iso);
@@ -7502,24 +7501,113 @@
FlagAndPersistent object_a, object_b;
+ intptr_t big_heap_size;
+
{
v8::HandleScope handle_scope(iso);
- object_a.handle.Reset(iso, v8::Object::New(iso));
- object_b.handle.Reset(iso, v8::Object::New(iso));
+ Local<Object> a(v8::Object::New(iso));
+ Local<Object> b(v8::Object::New(iso));
+ object_a.handle.Reset(iso, a);
+ object_b.handle.Reset(iso, b);
+ if (interlinked) {
+ a->Set(v8_str("x"), b);
+ b->Set(v8_str("x"), a);
+ }
+ if (global_gc) {
+ CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
+ } else {
+ CcTest::heap()->CollectGarbage(i::NEW_SPACE);
+ }
+ // We are relying on this creating a big flag array and reserving the
space
+ // up front.
+ v8::Handle<Value> big_array = CompileRun("new Array(50000)");
+ a->Set(v8_str("y"), big_array);
+ big_heap_size = CcTest::heap()->SizeOfObjects();
}
object_a.flag = false;
object_b.flag = false;
- object_a.handle.SetWeak(&object_a, &DisposeAndSetFlag);
- object_b.handle.SetWeak(&object_b, &DisposeAndSetFlag);
+ object_a.handle.SetPhantom(&object_a, &SetFlag);
+ object_b.handle.SetPhantom(&object_b, &SetFlag);
CHECK(!object_b.handle.IsIndependent());
object_a.handle.MarkIndependent();
object_b.handle.MarkIndependent();
CHECK(object_b.handle.IsIndependent());
- CcTest::heap()->CollectGarbage(i::NEW_SPACE);
+ if (global_gc) {
+ CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
+ } else {
+ CcTest::heap()->CollectGarbage(i::NEW_SPACE);
+ }
+ // A single GC should be enough to reclaim the memory, since we are using
+ // phantom handles.
+ CHECK_LT(CcTest::heap()->SizeOfObjects(), big_heap_size - 200000);
CHECK(object_a.flag);
CHECK(object_b.flag);
}
+
+
+THREADED_TEST(IndependentWeakHandle) {
+ IndependentWeakHandle(false, false);
+ IndependentWeakHandle(false, true);
+ IndependentWeakHandle(true, false);
+ IndependentWeakHandle(true, true);
+}
+
+
+static void ResetUseValueAndSetFlag(
+ const v8::WeakCallbackData<v8::Object, FlagAndPersistent>& data) {
+ // Blink will reset the handle, and then use the other handle, so they
+ // can't use the same backing slot.
+ data.GetParameter()->handle.Reset();
+ data.GetValue()->IsBoolean(); // Make sure the handle still works.
+ data.GetParameter()->flag = true;
+}
+
+
+static void ResetWeakHandle(bool global_gc) {
+ v8::Isolate* iso = CcTest::isolate();
+ v8::HandleScope scope(iso);
+ v8::Handle<Context> context = Context::New(iso);
+ Context::Scope context_scope(context);
+
+ FlagAndPersistent object_a, object_b;
+
+ {
+ v8::HandleScope handle_scope(iso);
+ Local<Object> a(v8::Object::New(iso));
+ Local<Object> b(v8::Object::New(iso));
+ object_a.handle.Reset(iso, a);
+ object_b.handle.Reset(iso, b);
+ if (global_gc) {
+ CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
+ } else {
+ CcTest::heap()->CollectGarbage(i::NEW_SPACE);
+ }
+ }
+
+ object_a.flag = false;
+ object_b.flag = false;
+ object_a.handle.SetWeak(&object_a, &ResetUseValueAndSetFlag);
+ object_b.handle.SetWeak(&object_b, &ResetUseValueAndSetFlag);
+ if (!global_gc) {
+ object_a.handle.MarkIndependent();
+ object_b.handle.MarkIndependent();
+ CHECK(object_b.handle.IsIndependent());
+ }
+ if (global_gc) {
+ CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
+ } else {
+ CcTest::heap()->CollectGarbage(i::NEW_SPACE);
+ }
+ CHECK(object_a.flag);
+ CHECK(object_b.flag);
+}
+
+
+THREADED_TEST(ResetWeakHandle) {
+ ResetWeakHandle(false);
+ ResetWeakHandle(true);
+}
static void InvokeScavenge() {
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
