Skip to content

Commit

Permalink
Change from storing method wrappers as hidden ivars on the appropriat…
Browse files Browse the repository at this point in the history
…e Ruby class to storing them in a global C++ unordered_map. This has several advantages:

* Removes some circular dependencies from lowel level detail code -> Data_Type/Data_Object -> low level details code
* Avoids creating a new Data_Type<T> for each wrapped method_data
* Avoids overhead of wrapping method_data in Ruby
* Avoids the Ruby garbage collector
  • Loading branch information
cfis committed Jan 30, 2021
1 parent 75854a4 commit 819515b
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 107 deletions.
11 changes: 1 addition & 10 deletions rice/Data_Type.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -378,16 +378,7 @@ define_iterator(Iterator_Return_T(U::* begin)(), Iterator_Return_T(U::* end)(),
{
using Iterator_T = detail::Iterator<U, Iterator_Return_T>;
Iterator_T* iterator = new Iterator_T(begin, end);

Data_Type<Iterator_T> iterator_klass;
Data_Object<Iterator_T> iteratorWrapper(iterator, iterator_klass);

detail::define_method_with_data(
Data_Type<T>::klass(),
name,
(RUBY_METHOD_FUNC)iterator->call,
0,
iteratorWrapper);
detail::MethodData::define_method(Data_Type<T>::klass(), name, (RUBY_METHOD_FUNC)iterator->call, 0, iterator);

return *this;
}
Expand Down
4 changes: 2 additions & 2 deletions rice/detail/Iterator.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ namespace Rice
inline VALUE Iterator<T, Iterator_Return_T>::
call(VALUE self)
{
VALUE data = detail::method_data();
Data_Object<Iterator> iterator(data, Data_Type<Iterator>());
using Iterator_T = Iterator<T, Iterator_Return_T>;
Iterator_T* iterator = std::any_cast<Iterator_T*>(detail::MethodData::data());
return iterator->operator()(self);
}

Expand Down
1 change: 1 addition & 0 deletions rice/detail/Wrapped_Function.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,5 @@ using NativeTypes = typename std::conditional_t<std::is_same_v<Receiver_T, std::

} // Rice


#endif // Rice__detail__Wrapped_Function__hpp_
7 changes: 3 additions & 4 deletions rice/detail/Wrapped_Function.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "../ruby_try_catch.hpp"
#include "to_ruby_defn.hpp"
#include "../Class_defn.hpp"
#include "../Data_Object_defn.hpp"

namespace Rice
{
Expand All @@ -20,10 +19,10 @@ template<typename Function_T, typename Return_T, typename Receiver_T, typename..
VALUE Wrapped_Function<Function_T, Return_T, Receiver_T, Arg_Ts...>::
call(int argc, VALUE* argv, VALUE self)
{
using WrapperType = Wrapped_Function<Function_T, Return_T, Receiver_T, Arg_Ts...>;
using Wrapper_T = Wrapped_Function<Function_T, Return_T, Receiver_T, Arg_Ts...>;

Data_Object<WrapperType> data(detail::method_data());
WrapperType* wrapper = (WrapperType*)data.get();
std::any foo = detail::MethodData::data();
Wrapper_T* wrapper = std::any_cast<Wrapper_T*>(foo);
return wrapper->operator()(argc, argv, self);
}

Expand Down
32 changes: 20 additions & 12 deletions rice/detail/method_data.hpp
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
#ifndef Rice__detail__method_data__hpp
#define Rice__detail__method_data__hpp

#include <unordered_map>
#include <any>

#include "ruby.hpp"

namespace Rice
{

namespace detail
{

VALUE define_method_with_data(VALUE klass, ID id, VALUE (*cfunc)(ANYARGS), int arity, VALUE data);

VALUE method_data();

} // namespace detail

} // namespace Rice

namespace detail
{
class MethodData
{
public:
// Defines a new Ruby method and stores the Rice metadata about it
static void define_method(VALUE klass, ID id, VALUE(*cfunc)(ANYARGS), int arity, std::any data);

// Returns the Rice data for the currently active Ruby method
static std::any data();

private:
static size_t key(VALUE klass, ID id);
inline static std::unordered_map<size_t, std::any> methodWrappers_ = {};
};
}
}
#include "method_data.ipp"

#endif // Rice__detail__method_data__hpp
109 changes: 43 additions & 66 deletions rice/detail/method_data.ipp
Original file line number Diff line number Diff line change
@@ -1,33 +1,3 @@
// TODO: This is silly, autoconf...
/*#undef PACKAGE_NAME
#undef PACKAGE_STRING
#undef PACKAGE_TARNAME
#undef PACKAGE_VERSION
#include "../config.hpp"
*/

#define RICE_ID rb_intern("__rice__")

inline VALUE
Rice::detail::
method_data()
{
ID id;
VALUE klass;
if (!rb_frame_method_id_and_class(&id, &klass))
{
rb_raise(
rb_eRuntimeError,
"Cannot get method id and class for function");
}

if (rb_type(klass) == T_ICLASS) {
klass = rb_class_of(klass);
}

VALUE store = rb_ivar_get(klass, RICE_ID);
return (store == Qnil) ? Qnil : rb_ivar_get(store, id);
}

// Ruby 2.7 now includes a similarly named macro that uses templates to
// pick the right overload for the underlying function. That doesn't work
Expand All @@ -36,42 +6,49 @@ method_data()
// back to the C-API underneath again.
#undef rb_define_method_id

// Define a method and attach metadata about the method to its owning class.
// This is done by wrapping data in a VALUE (via Data_Object) and then
// adding it as a hidden ivar to a storage object (an instance of a Ruby
// object) that is itself added as a hidden ivar to the method's class.
//
// When Ruby makes a method call, it stores the class Object and method
// ID in the current stack frame. When Ruby calls into Rice, we grab
// the class and method ID from the stack frame, then pull the data out
// of the class. The data item is then used to determine how to convert
// arguments and return type, how to handle exceptions, etc.
//
inline VALUE
Rice::detail::
define_method_with_data(
VALUE klass, ID id, VALUE (*cfunc)(ANYARGS), int arity, VALUE data)
namespace Rice
{
VALUE store = rb_attr_get(klass, RICE_ID);

if (store == Qnil)
namespace detail
{
store = rb_obj_alloc(rb_cObject);
// store is stored in the instance variables table with
// name "__rice__".
// since "__rice__" does not have the @ prefix,
// so it can never be read at the Ruby level.
rb_ivar_set(klass, RICE_ID, store);
// Effective Java (2nd edition)
// https://stackoverflow.com/a/2634715
inline size_t MethodData::key(VALUE klass, ID id)
{
if (rb_type(klass) == T_ICLASS)
{
klass = rb_class_of(klass);
}

uint32_t prime = 53;
return (prime + klass) * prime + id;
}

inline std::any MethodData::data()
{
ID id;
VALUE klass;
if (!rb_frame_method_id_and_class(&id, &klass))
{
rb_raise(rb_eRuntimeError, "Cannot get method id and class for function");
}

auto iter = methodWrappers_.find(key(klass, id));
if (iter == methodWrappers_.end())
{
rb_raise(rb_eRuntimeError, "Could not find data for klass and method id");
}

return iter->second;
}

inline void
MethodData::define_method(VALUE klass, ID id, VALUE(*cfunc)(ANYARGS), int arity, std::any data)
{
// Define the method
rb_define_method_id(klass, id, cfunc, arity);

// Now store data about it
methodWrappers_[key(klass, id)] = data;
}
}

rb_ivar_set(store, id, data);

// Create the aliased method on the origin class
rb_define_method_id(
klass,
id,
cfunc,
arity);

return Qnil;
}
}
15 changes: 5 additions & 10 deletions rice/forward_declares.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,12 @@ inline void Rice::Module::define_method_and_auto_wrap(VALUE klass, Identifier na
Arguments* arguments)
{
auto* wrapper = detail::wrap_function(function, handler, arguments);
using WrapperType = std::remove_pointer_t<decltype(wrapper)>;
using Wrapper_T = typename std::remove_pointer_t<decltype(wrapper)>;

Data_Object<WrapperType> f(wrapper, rb_cObject);

Rice::protect(
detail::define_method_with_data,
klass,
name.id(),
RUBY_METHOD_FUNC(&WrapperType::call),
-1,
f);
Rice::protect(detail::MethodData::define_method, klass, name.id(),
RUBY_METHOD_FUNC(&Wrapper_T::call),
-1,
wrapper);
}

#endif // Rice__Forward_Declares__ipp_
5 changes: 2 additions & 3 deletions rice/rice.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "detail/protect.hpp"
#include "detail/to_ruby.hpp"
#include "detail/Ruby_Function.hpp"
#include "detail/Wrapped_Function.hpp"
#include "detail/wrap_Function.hpp"

#include "Arg_operators.hpp"
#include "Exception.hpp"
Expand All @@ -31,9 +33,6 @@

#include "Object.hpp"

#include "detail/Wrapped_Function.hpp"
#include "detail/wrap_Function.hpp"

#include "Builtin_Object.hpp"
#include "String.hpp"
#include "Array.hpp"
Expand Down

0 comments on commit 819515b

Please sign in to comment.