Commit 69402ad2 authored by Jacob Potter's avatar Jacob Potter

Include typeid tags as keys in the proxy cache map.

Fixes #179.
parent 638ff7bb
......@@ -221,9 +221,8 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
w.wl(s"friend $baseType;")
w.wl
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.wl(s"using JavaProxyCacheEntry::JavaProxyCacheEntry;")
w.wl(s"JavaProxy(JniType j);")
w.wl(s"~JavaProxy();")
w.wl
......
......@@ -307,7 +307,7 @@ struct JavaProxyCacheTraits {
};
extern template class 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,
......@@ -405,7 +405,7 @@ public:
// Cases 3 and 4.
assert(m_cppProxyClass);
return JniCppProxyCache::get(c, &newCppProxy);
return JniCppProxyCache::get(typeid(c), c, &newCppProxy);
}
......@@ -447,10 +447,10 @@ private:
* 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).
*/
template <typename S, typename = typename S::JavaProxy>
template <typename S, typename JavaProxy = typename S::JavaProxy>
jobject _unwrapJavaProxy(const std::shared_ptr<I> * c) const {
if (auto proxy = dynamic_cast<typename S::JavaProxy *>(c->get())) {
return proxy->JavaProxyCacheEntry::get().get();
if (auto proxy = dynamic_cast<JavaProxy *>(c->get())) {
return proxy->JavaProxyHandle<JavaProxy>::get().get();
} else {
return nullptr;
}
......@@ -484,16 +484,16 @@ private:
* Helpers for _fromJava above. We can only produce a C++-side proxy 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>
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");
return std::static_pointer_cast<typename S::JavaProxy>(JavaProxyCache::get(
j,
return std::static_pointer_cast<JavaProxy>(JavaProxyCache::get(
typeid(JavaProxy), j,
[] (const jobject & obj) -> std::pair<std::shared_ptr<void>, jobject> {
auto ret = std::make_shared<typename S::JavaProxy>(obj);
return { ret, ret->JavaProxyCacheEntry::get().get() };
auto ret = std::make_shared<JavaProxy>(obj);
return { ret, ret->JavaProxyHandle<JavaProxy>::get().get() };
}
));
}
......
......@@ -41,6 +41,7 @@ using CppProxyCache = ProxyCache<CppProxyCacheTraits>;
template <typename ObjcType, typename CppType>
ObjcType * get_cpp_proxy_impl(const std::shared_ptr<CppType> & cppRef) {
return CppProxyCache::get(
typeid(cppRef),
cppRef,
[] (const std::shared_ptr<void> & cppRef) -> std::pair<id, void *> {
return {
......
......@@ -42,6 +42,7 @@ using ObjcProxyCache = ProxyCache<ObjcProxyCacheTraits>;
template <typename CppType, typename ObjcType>
static std::shared_ptr<CppType> get_objc_proxy(ObjcType * objcRef) {
return std::static_pointer_cast<CppType>(ObjcProxyCache::get(
typeid(objcRef),
objcRef,
[] (const __strong id & objcRef) -> std::pair<std::shared_ptr<void>, __unsafe_unretained id> {
return {
......
......@@ -67,6 +67,8 @@ template <typename T> static inline bool is_expired(T* ptr) { return !ptr; }
*/
template <typename Traits>
class ProxyCache<Traits>::Pimpl {
using Key = std::pair<std::type_index, UnowningImplPointer>;
public:
/*
* Look up an object in the proxy cache, and create a new one if not found.
......@@ -74,10 +76,12 @@ public:
* 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.
*/
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);
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()) {
OwningProxyPointer existing_proxy = upgrade_weak(existing_proxy_iter->second);
if (existing_proxy) {
......@@ -89,16 +93,16 @@ public:
}
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;
}
/*
* 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);
auto it = m_mapping.find(impl_unowning);
auto it = m_mapping.find({tag, impl_unowning});
if (it != m_mapping.end()) {
// 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
......@@ -116,10 +120,20 @@ public:
}
private:
std::unordered_map<UnowningImplPointer,
WeakProxyPointer,
UnowningImplPointerHash,
UnowningImplPointerEqual> m_mapping;
struct KeyHash {
std::size_t operator()(const Key & k) const {
return k.first.hash_code() ^ UnowningImplPointerHash{}(k.second);
}
};
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;
// Only ProxyCache<Traits>::get_base() can allocate these objects.
......@@ -128,8 +142,10 @@ private:
};
template <typename Traits>
void ProxyCache<Traits>::cleanup(const std::shared_ptr<Pimpl> & base, UnowningImplPointer ptr) {
base->remove(ptr);
void ProxyCache<Traits>::cleanup(const std::shared_ptr<Pimpl> & base,
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> & {
}
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 {
return get_base()->get(impl, alloc);
return get_base()->get(tag, impl, alloc);
}
} // namespace djinni
......@@ -17,6 +17,7 @@
#pragma once
#include <functional>
#include <typeindex>
namespace djinni {
......@@ -71,10 +72,10 @@ template <typename T> static inline T * get_unowning(T * ptr) { return ptr; }
*
* Here's an overview of the structure:
*
* ______________
* WeakProxyPonter | | UnowningImplPointer
* - - - - - - - -| ProxyCache:: |- - - - - - - - - -
* | | Handle<T> | |
* ______________ std::pair<ImplType,
* WeakProxyPonter | | UnowningImplPointer>
* - - - - - - - -| ProxyCache |- - - - - - - - - -
* | | | |
* | |______________| |
* | |
* ____v____ ______________ ______________v__________
......@@ -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 )
* ( or cross-language ) ( owning type like id<Foo>, )
* ( 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>
class ProxyCache {
......@@ -111,15 +120,20 @@ public:
* managing C++ proxies for ObjC objects, OwningImplPointer would be `id`, and the C++
* proxy class `MyInterface` which wraps `@protocol DBMyInterface` would contain a
* `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 {
public:
template <typename... Args> Handle(Args &&... args)
: m_cache(get_base()), m_obj(std::forward<Args>(args)...) {}
Handle(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; }
......@@ -156,10 +170,14 @@ public:
* 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.
*/
static OwningProxyPointer get(const OwningImplPointer & impl, AllocatorFunction * alloc);
static OwningProxyPointer get(const std::type_index &,
const OwningImplPointer & impl,
AllocatorFunction * alloc);
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();
};
......
......@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<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:
JavaProxy(JniType j);
......
......@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<NativeExternInterface2>;
friend ::djinni::JniInterface<::ExternInterface2, NativeExternInterface2>;
class JavaProxy final : ::djinni::JavaProxyCacheEntry, public ::ExternInterface2
class JavaProxy final : ::djinni::JavaProxyHandle<JavaProxy>, public ::ExternInterface2
{
public:
JavaProxy(JniType j);
......
......@@ -27,7 +27,7 @@ private:
friend ::djinni::JniClass<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:
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