From 79e817aac410422355549cfe1b8298509acf309d Mon Sep 17 00:00:00 2001 From: "dslomov@chromium.org" Date: Wed, 12 Mar 2014 11:40:40 +0000 Subject: [PATCH] Revert "Implement PersistentValueMap, a map that stores UniquePersistent values." and "Win64 fix for r19833." This reverts commits r19833 and r19837 for breaking Windows tests (test-api/PersistentValueMap). TBR=vogelheim@chromium.org,dcarney@chromium.org Review URL: https://codereview.chromium.org/197173002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19839 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 201 ++---------------------------------------------- src/api.cc | 4 +- src/global-handles.cc | 8 +- src/global-handles.h | 2 +- test/cctest/test-api.cc | 109 -------------------------- 5 files changed, 11 insertions(+), 313 deletions(-) diff --git a/include/v8.h b/include/v8.h index 9bda3dc..12a0b28 100644 --- a/include/v8.h +++ b/include/v8.h @@ -140,7 +140,6 @@ class ObjectOperationDescriptor; class RawOperationDescriptor; class CallHandlerHelper; class EscapableHandleScope; -template class ReturnValue; namespace internal { class Arguments; @@ -413,7 +412,6 @@ template class Local : public Handle { template friend class internal::CustomArguments; friend class HandleScope; friend class EscapableHandleScope; - template friend class PersistentValueMap; V8_INLINE static Local New(Isolate* isolate, T* that); }; @@ -529,11 +527,7 @@ template class PersistentBase { P* parameter, typename WeakCallbackData::Callback callback); - template - V8_INLINE P* ClearWeak(); - - // TODO(dcarney): remove this. - V8_INLINE void ClearWeak() { ClearWeak(); } + V8_INLINE void ClearWeak(); /** * Marks the reference to this object independent. Garbage collector is free @@ -582,7 +576,6 @@ template class PersistentBase { template friend class UniquePersistent; template friend class PersistentBase; template friend class ReturnValue; - template friend class PersistentValueMap; friend class Object; explicit V8_INLINE PersistentBase(T* val) : val_(val) {} @@ -751,7 +744,7 @@ class UniquePersistent : public PersistentBase { }; public: - /** + /** * A UniquePersistent with no storage cell. */ V8_INLINE UniquePersistent() : PersistentBase(0) { } @@ -789,7 +782,6 @@ class UniquePersistent : public PersistentBase { template V8_INLINE UniquePersistent& operator=(UniquePersistent rhs) { TYPE_CHECK(T, S); - this->Reset(); this->val_ = rhs.val_; rhs.val_ = 0; return *this; @@ -809,145 +801,6 @@ class UniquePersistent : public PersistentBase { }; -typedef uintptr_t PersistentContainerValue; -static const uintptr_t kPersistentContainerNotFound = 0; - -/** - * A map wrapper that allows using UniquePersistent as a mapped value. - * C++11 embedders don't need this class, as they can use UniquePersistent - * directly in std containers. - * - * The map relies on a backing map, whose type and accessors are described - * by the Traits class. The backing map will handle values of type - * PersistentContainerValue, with all conversion into and out of V8 - * handles being transparently handled by this class. - */ -template -class PersistentValueMap { - public: - V8_INLINE explicit PersistentValueMap(Isolate* isolate) : isolate_(isolate) {} - - V8_INLINE ~PersistentValueMap() { Clear(); } - - V8_INLINE Isolate* GetIsolate() { return isolate_; } - - /** - * Return size of the map. - */ - V8_INLINE size_t Size() { return Traits::Size(&impl_); } - - /** - * Get value stored in map. - */ - V8_INLINE Local Get(const K& key) { - return Local::New(isolate_, FromVal(Traits::Get(&impl_, key))); - } - - /** - * Check whether a value is contained in the map. - */ - V8_INLINE bool Contains(const K& key) { - return Traits::Get(&impl_, key) != 0; - } - - /** - * Get value stored in map and set it in returnValue. - * Return true if a value was found. - */ - V8_INLINE bool SetReturnValue(const K& key, - ReturnValue& returnValue); - - /** - * Call Isolate::SetReference with the given parent and the map value. - */ - V8_INLINE void SetReference(const K& key, - const v8::Persistent& parent) { - GetIsolate()->SetReference( - reinterpret_cast(parent.val_), - reinterpret_cast(FromVal(Traits::Get(&impl_, key)))); - } - - /** - * Put value into map. Depending on Traits::kIsWeak, the value will be held - * by the map strongly or weakly. - * Returns old value as UniquePersistent. - */ - UniquePersistent Set(const K& key, Local value) { - UniquePersistent persistent(isolate_, value); - return SetUnique(key, &persistent); - } - - /** - * Put value into map, like Set(const K&, Local). - */ - UniquePersistent Set(const K& key, UniquePersistent value) { - return SetUnique(key, &value); - } - - /** - * Return value for key and remove it from the map. - */ - V8_INLINE UniquePersistent Remove(const K& key) { - return Release(Traits::Remove(&impl_, key)).Pass(); - } - - /** - * Traverses the map repeatedly, - * in case side effects of disposal cause insertions. - **/ - void Clear(); - - private: - PersistentValueMap(PersistentValueMap&); - void operator=(PersistentValueMap&); - - /** - * Put the value into the map, and set the 'weak' callback when demanded - * by the Traits class. - */ - UniquePersistent SetUnique(const K& key, UniquePersistent* persistent) { - if (Traits::kIsWeak) { - Local value(Local::New(isolate_, *persistent)); - persistent->template SetWeak( - Traits::WeakCallbackParameter(&impl_, key, value), WeakCallback); - } - PersistentContainerValue old_value = - Traits::Set(&impl_, key, ClearAndLeak(persistent)); - return Release(old_value).Pass(); - } - - static void WeakCallback( - const WeakCallbackData& data); - V8_INLINE static V* FromVal(PersistentContainerValue v) { - return reinterpret_cast(v); - } - V8_INLINE static PersistentContainerValue ClearAndLeak( - UniquePersistent* persistent) { - V* v = persistent->val_; - persistent->val_ = 0; - return reinterpret_cast(v); - } - - /** - * Return a container value as UniquePersistent and make sure the weak - * callback is properly disposed of. All remove functionality should go - * through this. - */ - V8_INLINE static UniquePersistent Release(PersistentContainerValue v) { - UniquePersistent p; - p.val_ = FromVal(v); - if (Traits::kIsWeak && !p.IsEmpty()) { - Traits::DisposeCallbackData( - p.template ClearWeak()); - } - return p.Pass(); - } - - Isolate* isolate_; - typename Traits::Impl impl_; -}; - - /** * A stack-allocated class that governs a number of local handles. * After a handle scope has been created, all local handles will be @@ -2535,8 +2388,6 @@ class ReturnValue { template friend class ReturnValue; template friend class FunctionCallbackInfo; template friend class PropertyCallbackInfo; - template friend class PersistentValueMap; - V8_INLINE void SetInternal(internal::Object* value) { *value_ = value; } V8_INLINE internal::Object* GetDefaultValue(); V8_INLINE explicit ReturnValue(internal::Object** slot); internal::Object** value_; @@ -4384,8 +4235,6 @@ class V8_EXPORT Isolate { void SetEventLogger(LogEventCallback that); private: - template friend class PersistentValueMap; - Isolate(); Isolate(const Isolate&); ~Isolate(); @@ -4980,7 +4829,7 @@ class V8_EXPORT V8 { static void MakeWeak(internal::Object** global_handle, void* data, WeakCallback weak_callback); - static void* ClearWeak(internal::Object** global_handle); + static void ClearWeak(internal::Object** global_handle); static void Eternalize(Isolate* isolate, Value* handle, int* index); @@ -5893,10 +5742,8 @@ void PersistentBase::SetWeak( template -template -P* PersistentBase::ClearWeak() { - return reinterpret_cast( - V8::ClearWeak(reinterpret_cast(this->val_))); +void PersistentBase::ClearWeak() { + V8::ClearWeak(reinterpret_cast(this->val_)); } @@ -5949,44 +5796,6 @@ uint16_t PersistentBase::WrapperClassId() const { } -template -bool PersistentValueMap::SetReturnValue(const K& key, - ReturnValue& returnValue) { - PersistentContainerValue value = Traits::Get(&impl_, key); - bool hasValue = value != 0; - if (hasValue) { - returnValue.SetInternal( - *reinterpret_cast(FromVal(value))); - } - return hasValue; -} - -template -void PersistentValueMap::Clear() { - typedef typename Traits::Iterator It; - HandleScope handle_scope(isolate_); - // TODO(dcarney): figure out if this swap and loop is necessary. - while (!Traits::Empty(&impl_)) { - typename Traits::Impl impl; - Traits::Swap(impl_, impl); - for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) { - Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl, - Traits::Key(i)); - } - } -} - - -template -void PersistentValueMap::WeakCallback( - const WeakCallbackData& data) { - typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data); - K key = Traits::KeyFromWeakCallbackData(data); - PersistentContainerValue value = Traits::Remove(impl, key); - Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key); -} - - template ReturnValue::ReturnValue(internal::Object** slot) : value_(slot) {} diff --git a/src/api.cc b/src/api.cc index 4306967..a6ff52c 100644 --- a/src/api.cc +++ b/src/api.cc @@ -560,8 +560,8 @@ void V8::MakeWeak(i::Object** object, } -void* V8::ClearWeak(i::Object** obj) { - return i::GlobalHandles::ClearWeakness(obj); +void V8::ClearWeak(i::Object** obj) { + i::GlobalHandles::ClearWeakness(obj); } diff --git a/src/global-handles.cc b/src/global-handles.cc index e06f794..211bb1c 100644 --- a/src/global-handles.cc +++ b/src/global-handles.cc @@ -235,12 +235,10 @@ class GlobalHandles::Node { weak_callback_ = weak_callback; } - void* ClearWeakness() { + void ClearWeakness() { ASSERT(state() != FREE); - void* p = parameter(); set_state(NORMAL); set_parameter(NULL); - return p; } bool PostGarbageCollectionProcessing(Isolate* isolate) { @@ -504,8 +502,8 @@ void GlobalHandles::MakeWeak(Object** location, } -void* GlobalHandles::ClearWeakness(Object** location) { - return Node::FromLocation(location)->ClearWeakness(); +void GlobalHandles::ClearWeakness(Object** location) { + Node::FromLocation(location)->ClearWeakness(); } diff --git a/src/global-handles.h b/src/global-handles.h index 13fc111..f46a22a 100644 --- a/src/global-handles.h +++ b/src/global-handles.h @@ -161,7 +161,7 @@ class GlobalHandles { } // Clear the weakness of a global handle. - static void* ClearWeakness(Object** location); + static void ClearWeakness(Object** location); // Clear the weakness of a global handle. static void MarkIndependent(Object** location); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 4ee0eb7..5bb509a 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -3444,115 +3444,6 @@ THREADED_TEST(UniquePersistent) { } -template -class StdPersistentValueMapTraits { - public: - static const bool kIsWeak = is_weak; - typedef v8::PersistentContainerValue VInt; - typedef std::map Impl; - struct WeakCallbackDataType { - Impl* impl; - K key; - }; - typedef typename Impl::iterator Iterator; - static bool Empty(Impl* impl) { return impl->empty(); } - static size_t Size(Impl* impl) { return impl->size(); } - static void Swap(Impl& a, Impl& b) { std::swap(a, b); } // NOLINT - static Iterator Begin(Impl* impl) { return impl->begin(); } - static Iterator End(Impl* impl) { return impl->end(); } - static K Key(Iterator it) { return it->first; } - static VInt Value(Iterator it) { return it->second; } - static VInt Set(Impl* impl, K key, VInt value) { - std::pair res = impl->insert(std::make_pair(key, value)); - VInt old_value = v8::kPersistentContainerNotFound; - if (!res.second) { - old_value = res.first->second; - res.first->second = value; - } - return old_value; - } - static VInt Get(Impl* impl, K key) { - Iterator it = impl->find(key); - if (it == impl->end()) return v8::kPersistentContainerNotFound; - return it->second; - } - static VInt Remove(Impl* impl, K key) { - Iterator it = impl->find(key); - if (it == impl->end()) return v8::kPersistentContainerNotFound; - impl->erase(it); - return it->second; - } - static void Dispose(v8::Isolate* isolate, v8::UniquePersistent value, - Impl* impl, K key) {} - static WeakCallbackDataType* WeakCallbackParameter( - Impl* impl, const K& key, Local value) { - WeakCallbackDataType* data = new WeakCallbackDataType; - data->impl = impl; - data->key = key; - return data; - } - static Impl* ImplFromWeakCallbackData( - const v8::WeakCallbackData& data) { - return data.GetParameter()->impl; - } - static K KeyFromWeakCallbackData( - const v8::WeakCallbackData& data) { - return data.GetParameter()->key; - } - static void DisposeCallbackData(WeakCallbackDataType* data) { - delete data; - } -}; - - -template -static void TestPersistentValueMap() { - LocalContext env; - v8::Isolate* isolate = env->GetIsolate(); - typedef v8::PersistentValueMap > Map; - Map map(isolate); - v8::internal::GlobalHandles* global_handles = - reinterpret_cast(isolate)->global_handles(); - int initial_handle_count = global_handles->global_handles_count(); - CHECK_EQ(0, static_cast(map.Size())); - { - HandleScope scope(isolate); - Local obj = map.Get(7); - CHECK(obj.IsEmpty()); - Local expected = v8::Object::New(isolate); - map.Set(7, expected); - CHECK_EQ(1, static_cast(map.Size())); - obj = map.Get(7); - CHECK_EQ(expected, obj); - v8::UniquePersistent removed = map.Remove(7); - CHECK_EQ(0, static_cast(map.Size())); - CHECK(expected == removed); - removed = map.Remove(7); - CHECK(removed.IsEmpty()); - map.Set(8, expected); - CHECK_EQ(1, static_cast(map.Size())); - map.Set(8, expected); - CHECK_EQ(1, static_cast(map.Size())); - } - CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count()); - if (is_weak) { - reinterpret_cast(isolate)->heap()-> - CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask); - } else { - map.Clear(); - } - CHECK_EQ(0, static_cast(map.Size())); - CHECK_EQ(initial_handle_count, global_handles->global_handles_count()); -} - - -TEST(PersistentValueMap) { - TestPersistentValueMap(); - TestPersistentValueMap(); -} - - THREADED_TEST(GlobalHandleUpcast) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope scope(isolate); -- 2.7.4