Commit 4fbab35f authored by Andrew Twyman's avatar Andrew Twyman

Merge pull request #185 from j4cbo/typetag

Include typeid tags as keys in the proxy cache map.
parents 638ff7bb 69402ad2
...@@ -221,9 +221,8 @@ class JNIGenerator(spec: Spec) extends Generator(spec) { ...@@ -221,9 +221,8 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
w.wl(s"friend $baseType;") w.wl(s"friend $baseType;")
w.wl w.wl
if (i.ext.java) { if (i.ext.java) {
w.wl(s"class JavaProxy final : ::djinni::JavaProxyCacheEntry, public $cppSelf").bracedSemi { w.wl(s"class JavaProxy final : ::djinni::JavaProxyHandle<JavaProxy>, public $cppSelf").bracedSemi {
w.wlOutdent(s"public:") w.wlOutdent(s"public:")
// w.wl(s"using JavaProxyCacheEntry::JavaProxyCacheEntry;")
w.wl(s"JavaProxy(JniType j);") w.wl(s"JavaProxy(JniType j);")
w.wl(s"~JavaProxy();") w.wl(s"~JavaProxy();")
w.wl w.wl
......
...@@ -307,7 +307,7 @@ struct JavaProxyCacheTraits { ...@@ -307,7 +307,7 @@ struct JavaProxyCacheTraits {
}; };
extern template class ProxyCache<JavaProxyCacheTraits>; extern template class ProxyCache<JavaProxyCacheTraits>;
using JavaProxyCache = ProxyCache<JavaProxyCacheTraits>; using JavaProxyCache = ProxyCache<JavaProxyCacheTraits>;
using JavaProxyCacheEntry = JavaProxyCache::Handle<GlobalRef<jobject>>; template <typename T> using JavaProxyHandle = JavaProxyCache::Handle<GlobalRef<jobject>, T>;
/* /*
* Cache for CppProxy objects. This is the inverse of the JavaProxyCache mechanism above, * Cache for CppProxy objects. This is the inverse of the JavaProxyCache mechanism above,
...@@ -405,7 +405,7 @@ public: ...@@ -405,7 +405,7 @@ public:
// Cases 3 and 4. // Cases 3 and 4.
assert(m_cppProxyClass); assert(m_cppProxyClass);
return JniCppProxyCache::get(c, &newCppProxy); return JniCppProxyCache::get(typeid(c), c, &newCppProxy);
} }
...@@ -447,10 +447,10 @@ private: ...@@ -447,10 +447,10 @@ private:
* Helpers for _toJava above. The possibility that an object is already a C++-side proxy * Helpers for _toJava above. The possibility that an object is already a C++-side proxy
* only exists if the code generator emitted one (if Self::JavaProxy exists). * only exists if the code generator emitted one (if Self::JavaProxy exists).
*/ */
template <typename S, typename = typename S::JavaProxy> template <typename S, typename JavaProxy = typename S::JavaProxy>
jobject _unwrapJavaProxy(const std::shared_ptr<I> * c) const { jobject _unwrapJavaProxy(const std::shared_ptr<I> * c) const {
if (auto proxy = dynamic_cast<typename S::JavaProxy *>(c->get())) { if (auto proxy = dynamic_cast<JavaProxy *>(c->get())) {
return proxy->JavaProxyCacheEntry::get().get(); return proxy->JavaProxyHandle<JavaProxy>::get().get();
} else { } else {
return nullptr; return nullptr;
} }
...@@ -484,16 +484,16 @@ private: ...@@ -484,16 +484,16 @@ private:
* Helpers for _fromJava above. We can only produce a C++-side proxy if the code generator * Helpers for _fromJava above. We can only produce a C++-side proxy if the code generator
* emitted one (if Self::JavaProxy exists). * emitted one (if Self::JavaProxy exists).
*/ */
template <typename S, typename = typename S::JavaProxy> template <typename S, typename JavaProxy = typename S::JavaProxy>
std::shared_ptr<I> _getJavaProxy(jobject j) const { std::shared_ptr<I> _getJavaProxy(jobject j) const {
static_assert(std::is_base_of<JavaProxyCacheEntry, typename S::JavaProxy>::value, static_assert(std::is_base_of<JavaProxyHandle<JavaProxy>, JavaProxy>::value,
"JavaProxy must derive from JavaProxyCacheEntry"); "JavaProxy must derive from JavaProxyCacheEntry");
return std::static_pointer_cast<typename S::JavaProxy>(JavaProxyCache::get( return std::static_pointer_cast<JavaProxy>(JavaProxyCache::get(
j, typeid(JavaProxy), j,
[] (const jobject & obj) -> std::pair<std::shared_ptr<void>, jobject> { [] (const jobject & obj) -> std::pair<std::shared_ptr<void>, jobject> {
auto ret = std::make_shared<typename S::JavaProxy>(obj); auto ret = std::make_shared<JavaProxy>(obj);
return { ret, ret->JavaProxyCacheEntry::get().get() }; return { ret, ret->JavaProxyHandle<JavaProxy>::get().get() };
} }
)); ));
} }
......
...@@ -41,6 +41,7 @@ using CppProxyCache = ProxyCache<CppProxyCacheTraits>; ...@@ -41,6 +41,7 @@ using CppProxyCache = ProxyCache<CppProxyCacheTraits>;
template <typename ObjcType, typename CppType> template <typename ObjcType, typename CppType>
ObjcType * get_cpp_proxy_impl(const std::shared_ptr<CppType> & cppRef) { ObjcType * get_cpp_proxy_impl(const std::shared_ptr<CppType> & cppRef) {
return CppProxyCache::get( return CppProxyCache::get(
typeid(cppRef),
cppRef, cppRef,
[] (const std::shared_ptr<void> & cppRef) -> std::pair<id, void *> { [] (const std::shared_ptr<void> & cppRef) -> std::pair<id, void *> {
return { return {
......
...@@ -42,6 +42,7 @@ using ObjcProxyCache = ProxyCache<ObjcProxyCacheTraits>; ...@@ -42,6 +42,7 @@ using ObjcProxyCache = ProxyCache<ObjcProxyCacheTraits>;
template <typename CppType, typename ObjcType> template <typename CppType, typename ObjcType>
static std::shared_ptr<CppType> get_objc_proxy(ObjcType * objcRef) { static std::shared_ptr<CppType> get_objc_proxy(ObjcType * objcRef) {
return std::static_pointer_cast<CppType>(ObjcProxyCache::get( return std::static_pointer_cast<CppType>(ObjcProxyCache::get(
typeid(objcRef),
objcRef, objcRef,
[] (const __strong id & objcRef) -> std::pair<std::shared_ptr<void>, __unsafe_unretained id> { [] (const __strong id & objcRef) -> std::pair<std::shared_ptr<void>, __unsafe_unretained id> {
return { return {
......
...@@ -67,6 +67,8 @@ template <typename T> static inline bool is_expired(T* ptr) { return !ptr; } ...@@ -67,6 +67,8 @@ template <typename T> static inline bool is_expired(T* ptr) { return !ptr; }
*/ */
template <typename Traits> template <typename Traits>
class ProxyCache<Traits>::Pimpl { class ProxyCache<Traits>::Pimpl {
using Key = std::pair<std::type_index, UnowningImplPointer>;
public: public:
/* /*
* Look up an object in the proxy cache, and create a new one if not found. * Look up an object in the proxy cache, and create a new one if not found.
...@@ -74,10 +76,12 @@ public: ...@@ -74,10 +76,12 @@ public:
* This takes a function pointer, not an arbitrary functor, because we want to minimize * This takes a function pointer, not an arbitrary functor, because we want to minimize
* code size: this function should only be instantiated *once* per langauge direction. * code size: this function should only be instantiated *once* per langauge direction.
*/ */
OwningProxyPointer get(const OwningImplPointer & impl, AllocatorFunction * alloc) { OwningProxyPointer get(const std::type_index & tag,
const OwningImplPointer & impl,
AllocatorFunction * alloc) {
std::unique_lock<std::mutex> lock(m_mutex); std::unique_lock<std::mutex> lock(m_mutex);
UnowningImplPointer ptr = get_unowning(impl); UnowningImplPointer ptr = get_unowning(impl);
auto existing_proxy_iter = m_mapping.find(ptr); auto existing_proxy_iter = m_mapping.find({tag, ptr});
if (existing_proxy_iter != m_mapping.end()) { if (existing_proxy_iter != m_mapping.end()) {
OwningProxyPointer existing_proxy = upgrade_weak(existing_proxy_iter->second); OwningProxyPointer existing_proxy = upgrade_weak(existing_proxy_iter->second);
if (existing_proxy) { if (existing_proxy) {
...@@ -89,16 +93,16 @@ public: ...@@ -89,16 +93,16 @@ public:
} }
auto alloc_result = alloc(impl); auto alloc_result = alloc(impl);
m_mapping.emplace(alloc_result.second, alloc_result.first); m_mapping.emplace(Key{tag, alloc_result.second}, alloc_result.first);
return alloc_result.first; return alloc_result.first;
} }
/* /*
* Erase an object from the proxy cache. * Erase an object from the proxy cache.
*/ */
void remove(const UnowningImplPointer & impl_unowning) { void remove(const std::type_index & tag, const UnowningImplPointer & impl_unowning) {
std::unique_lock<std::mutex> lock(m_mutex); std::unique_lock<std::mutex> lock(m_mutex);
auto it = m_mapping.find(impl_unowning); auto it = m_mapping.find({tag, impl_unowning});
if (it != m_mapping.end()) { if (it != m_mapping.end()) {
// The entry in the map should already be expired: this is called from Handle's // The entry in the map should already be expired: this is called from Handle's
// destructor, so the proxy must already be gone. However, remove() does not // destructor, so the proxy must already be gone. However, remove() does not
...@@ -116,10 +120,20 @@ public: ...@@ -116,10 +120,20 @@ public:
} }
private: private:
std::unordered_map<UnowningImplPointer, struct KeyHash {
WeakProxyPointer, std::size_t operator()(const Key & k) const {
UnowningImplPointerHash, return k.first.hash_code() ^ UnowningImplPointerHash{}(k.second);
UnowningImplPointerEqual> m_mapping; }
};
struct KeyEqual {
bool operator()(const Key & lhs, const Key & rhs) const {
return lhs.first == rhs.first
&& UnowningImplPointerEqual{}(lhs.second, rhs.second);
}
};
std::unordered_map<Key, WeakProxyPointer, KeyHash, KeyEqual> m_mapping;
std::mutex m_mutex; std::mutex m_mutex;
// Only ProxyCache<Traits>::get_base() can allocate these objects. // Only ProxyCache<Traits>::get_base() can allocate these objects.
...@@ -128,8 +142,10 @@ private: ...@@ -128,8 +142,10 @@ private:
}; };
template <typename Traits> template <typename Traits>
void ProxyCache<Traits>::cleanup(const std::shared_ptr<Pimpl> & base, UnowningImplPointer ptr) { void ProxyCache<Traits>::cleanup(const std::shared_ptr<Pimpl> & base,
base->remove(ptr); const std::type_index & tag,
UnowningImplPointer ptr) {
base->remove(tag, ptr);
} }
/* /*
...@@ -150,9 +166,11 @@ auto ProxyCache<Traits>::get_base() -> const std::shared_ptr<Pimpl> & { ...@@ -150,9 +166,11 @@ auto ProxyCache<Traits>::get_base() -> const std::shared_ptr<Pimpl> & {
} }
template <typename Traits> template <typename Traits>
auto ProxyCache<Traits>::get(const OwningImplPointer & impl, AllocatorFunction * alloc) auto ProxyCache<Traits>::get(const std::type_index & tag,
const OwningImplPointer & impl,
AllocatorFunction * alloc)
-> OwningProxyPointer { -> OwningProxyPointer {
return get_base()->get(impl, alloc); return get_base()->get(tag, impl, alloc);
} }
} // namespace djinni } // namespace djinni
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#pragma once #pragma once
#include <functional> #include <functional>
#include <typeindex>
namespace djinni { namespace djinni {
...@@ -71,10 +72,10 @@ template <typename T> static inline T * get_unowning(T * ptr) { return ptr; } ...@@ -71,10 +72,10 @@ template <typename T> static inline T * get_unowning(T * ptr) { return ptr; }
* *
* Here's an overview of the structure: * Here's an overview of the structure:
* *
* ______________ * ______________ std::pair<ImplType,
* WeakProxyPonter | | UnowningImplPointer * WeakProxyPonter | | UnowningImplPointer>
* - - - - - - - -| ProxyCache:: |- - - - - - - - - - * - - - - - - - -| ProxyCache |- - - - - - - - - -
* | | Handle<T> | | * | | | |
* | |______________| | * | |______________| |
* | | * | |
* ____v____ ______________ ______________v__________ * ____v____ ______________ ______________v__________
...@@ -86,6 +87,14 @@ template <typename T> static inline T * get_unowning(T * ptr) { return ptr; } ...@@ -86,6 +87,14 @@ template <typename T> static inline T * get_unowning(T * ptr) { return ptr; }
* ( can be member, base, ) ( T is a generally a specific ) * ( can be member, base, ) ( T is a generally a specific )
* ( or cross-language ) ( owning type like id<Foo>, ) * ( or cross-language ) ( owning type like id<Foo>, )
* ( reference like jlong ) ( shared_ptr<Foo>, or GlobalRef ) * ( reference like jlong ) ( shared_ptr<Foo>, or GlobalRef )
*
* The cache contains a map from pair<ImplType, UnowningImplPointer>
* to WeakProxyPointer, allowing it to answer the question: "given this
* impl, do we already have a proxy in existance?"
*
* We use one map for all translated types, rather than a separate one for each type,
* to minimize duplication of code and make it so the unordered_map is as contained as
* possible.
*/ */
template <typename Traits> template <typename Traits>
class ProxyCache { class ProxyCache {
...@@ -111,15 +120,20 @@ public: ...@@ -111,15 +120,20 @@ public:
* managing C++ proxies for ObjC objects, OwningImplPointer would be `id`, and the C++ * managing C++ proxies for ObjC objects, OwningImplPointer would be `id`, and the C++
* proxy class `MyInterface` which wraps `@protocol DBMyInterface` would contain a * proxy class `MyInterface` which wraps `@protocol DBMyInterface` would contain a
* `Handle<id<DBMyInterface>>`. * `Handle<id<DBMyInterface>>`.
*
* TagType should be the same type that was passed in to `get()` when this handle was
* created. Normally this is the same as T (a specialized OwningImplPointer), but in
* cases like Java where all object types are uniformly represented as `jobject` in C++,
* another type may be used.
*/ */
template <class T> template <typename T, typename TagType = T>
class Handle { class Handle {
public: public:
template <typename... Args> Handle(Args &&... args) template <typename... Args> Handle(Args &&... args)
: m_cache(get_base()), m_obj(std::forward<Args>(args)...) {} : m_cache(get_base()), m_obj(std::forward<Args>(args)...) {}
Handle(const Handle &) = delete; Handle(const Handle &) = delete;
Handle & operator=(const Handle &) = delete; Handle & operator=(const Handle &) = delete;
~Handle() { if (m_obj) cleanup(m_cache, get_unowning(m_obj)); } ~Handle() { if (m_obj) cleanup(m_cache, typeid(TagType), get_unowning(m_obj)); }
void assign(const T & obj) { m_obj = obj; } void assign(const T & obj) { m_obj = obj; }
...@@ -156,10 +170,14 @@ public: ...@@ -156,10 +170,14 @@ public:
* Return the existing proxy for `impl`, if any. If not, create one by calling `alloc`, * Return the existing proxy for `impl`, if any. If not, create one by calling `alloc`,
* store a weak reference to it in the proxy cache, and return it. * store a weak reference to it in the proxy cache, and return it.
*/ */
static OwningProxyPointer get(const OwningImplPointer & impl, AllocatorFunction * alloc); static OwningProxyPointer get(const std::type_index &,
const OwningImplPointer & impl,
AllocatorFunction * alloc);
private: private:
static void cleanup(const std::shared_ptr<Pimpl> &, UnowningImplPointer); static void cleanup(const std::shared_ptr<Pimpl> &,
const std::type_index &,
UnowningImplPointer);
static const std::shared_ptr<Pimpl> & get_base(); static const std::shared_ptr<Pimpl> & get_base();
}; };
......
...@@ -27,7 +27,7 @@ private: ...@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<NativeClientInterface>; friend ::djinni::JniClass<NativeClientInterface>;
friend ::djinni::JniInterface<::testsuite::ClientInterface, NativeClientInterface>; friend ::djinni::JniInterface<::testsuite::ClientInterface, NativeClientInterface>;
class JavaProxy final : ::djinni::JavaProxyCacheEntry, public ::testsuite::ClientInterface class JavaProxy final : ::djinni::JavaProxyHandle<JavaProxy>, public ::testsuite::ClientInterface
{ {
public: public:
JavaProxy(JniType j); JavaProxy(JniType j);
......
...@@ -27,7 +27,7 @@ private: ...@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<NativeExternInterface2>; friend ::djinni::JniClass<NativeExternInterface2>;
friend ::djinni::JniInterface<::ExternInterface2, NativeExternInterface2>; friend ::djinni::JniInterface<::ExternInterface2, NativeExternInterface2>;
class JavaProxy final : ::djinni::JavaProxyCacheEntry, public ::ExternInterface2 class JavaProxy final : ::djinni::JavaProxyHandle<JavaProxy>, public ::ExternInterface2
{ {
public: public:
JavaProxy(JniType j); JavaProxy(JniType j);
......
...@@ -27,7 +27,7 @@ private: ...@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<NativeUserToken>; friend ::djinni::JniClass<NativeUserToken>;
friend ::djinni::JniInterface<::testsuite::UserToken, NativeUserToken>; friend ::djinni::JniInterface<::testsuite::UserToken, NativeUserToken>;
class JavaProxy final : ::djinni::JavaProxyCacheEntry, public ::testsuite::UserToken class JavaProxy final : ::djinni::JavaProxyHandle<JavaProxy>, public ::testsuite::UserToken
{ {
public: public:
JavaProxy(JniType j); JavaProxy(JniType j);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment