Efficiency issue in boxing std::function objects

[camio]

The code for boxing callables looks something like this:

rust::Box<::rust::Dyn<::rust::Fn<::int32_t, ::int32_t>>>
rust::Box<::rust::Dyn<::rust::Fn<::int32_t, ::int32_t>>>::make_box(
    ::std::function<::int32_t(::int32_t)> f) {
  auto data = new ::std::function<::int32_t(::int32_t)>(f);
  rust::Box<::rust::Dyn<::rust::Fn<::int32_t, ::int32_t>>> o;
  ::rust::__zngur_internal_assume_init(o);
  __zngur_i32_i32_c11(
      reinterpret_cast<uint8_t *>(data),
      [](uint8_t *d) {
        delete reinterpret_cast<::std::function<::int32_t(::int32_t)> *>(d);
      },
      [](uint8_t *d, uint8_t *i0, uint8_t *o) {
        auto dd = reinterpret_cast<::std::function<::int32_t(::int32_t)> *>(d);
        ::int32_t oo =
            (*dd)(::rust::__zngur_internal_move_from_rust<::int32_t>(i0));
        ::rust::__zngur_internal_move_to_rust<::int32_t>(o, oo);
      },
      ::rust::__zngur_internal_data_ptr(o));
  return o;
}

There are potentially three allocations here. The first is when the std::function argument is initialized, the second happens when data is allocated, and the third is when *data is initialized using its copy constructor.

An easy way to avoid one of these allocations is to use std::move when using new like this:

  auto data = new ::std::function<::int32_t(::int32_t)>( std::move(f) );

However, I think it may be better to use a template interface:

template<typename... Args>
rust::Box<::rust::Dyn<::rust::Fn<::int32_t, ::int32_t>>>
rust::Box<::rust::Dyn<::rust::Fn<::int32_t, ::int32_t>>>::make_box( Args&&... args) {
  auto data = new ::std::function<::int32_t(::int32_t)>(::std::forward<Args>(args)...);
  // ...
  return o;
}

This way you avoid the move and, once C++26 drops, you can switch to std::copyable_function which will allow your callers to use std::in_place_type_t as an argument and potentially gain further efficiency gains.

[camio]

Thinking more about this, it would be nice if make_box's semantics were consistent for both dyn Fn and dyn T. One way to do this would be to follow the copyable_function pattern, which would go something like this...

struct Bar : Foo {
    Bar(int i);
    //...
};

Box<Dyn<Foo>> b1( Bar(3) ); // Initialize with an object
Box<Dyn<Foo>> b2( std::inplace_type<Foo>, 3 ); // initialize "in place" with the object's constructor

struct SomeFunctor {
    SomeFunctor(int i);
    Unit operator()();
}

Box<Dyn<Fn<Unit>>> b1(SomeFunctor(3)); // Initialize with an object
Box<Dyn<Fn<Unit>>> b2( std::inplace_type<SomeFunctor>, 3 ); // Initialize in place" with the object's constructor
Box<Dyn<Fn<Unit>>> b3([](){ return Unit(); }); // Initialize with a (lambda) object

What do you think?

[hkalbasi]

I guess two allocation is needed in both dyn T and dyn Fn cases, one for the vtable in the C++ side and one for the dyn in Rust side. By using std::function<...T>* I'm forcing the third allocation, since std::function<...T>* is a heap pointer to a (potentially, if 32 byte is not enough for storing the lambda) heap pointer. So we may want to store std::function<...T> directly instead of using std::function<...T>* and casting it to uint8_t*, but I'm not sure if it is possible to do in a portable way (I guess it is possible by defining my own std::function).

However, I think it may be better to use a template interface:

I didn't understand how using template could reduce the number of allocations. It still have one potential allocation in the constructor of std::function, and one for the new. If the goal is just eliminating the move, I would expect a sufficiently smart compiler could inline and eventually remove the move, but if it is not the case in the current compilers, using the template makes sense. And your second point on consistency and ergonomics makes sense so I will do it but I still like to know if it also has performance impact.

[camio]

However, I think it may be better to use a template interface:

I didn't understand how using template could reduce the number of allocations. It still have one potential allocation in the constructor of std::function, and one for the new. If the goal is just eliminating the move, I would expect a sufficiently smart compiler could inline and eventually remove the move, but if it is not the case in the current compilers, using the template makes sense. And your second point on consistency and ergonomics makes sense so I will do it but I still like to know if it also has performance impact.

You're correct, using a function template will not avoid allocations above what is done with the std::move version. Avoiding the move, however, could have real performance implications when, for example, the underlying object is a std::array. But, yeah, I agree that the consistency argument made in #29 (comment) trumps the other considerations.