Re: Capy Review

On 6/25/26 10:23, Vinnie Falco wrote:
>> I know it's deliberate.  That doesn't make it any less restrictive.
>>
> 
> Capy is being evaluated here by itself, outside the context of Corosio, and
> for a use-case it doesn't claim.

The thing about i/o code is that it's never *just* i/o.  Information 
sent to a file or a network connection has to come from somewhere, and 
information from a file or network connection needs to be handled 
somehow.  An i/o system needs to interact with non-i/o code, preferably 
in the same coroutine without unnecessary context switching.

My choice to go outside Capy's comfort zone was very much deliberate, 
because every application developer who uses Capy will eventually need 
to step outside that comfort zone.

>> I like my approach better, although I don't know how viable the to_io_awaitable
>> function is.  Or why Capy doesn't provide it, if it is.
>>
> 
> Capy doesn't provide to_io_awaitable() because it can't be implemented
> correctly for arbitrary awaitables. The wrapper would have to call resume()
> on the right executor in order to maintain the execution guarantees, and it
> can't know which executor that is without domain knowledge about how the
> foreign operation completes.

The foreign operation completes in these ways:
   - By calling std::coroutine_handle::resume, which is the standard 
interface for resuming a suspended coroutine.
   - By not suspending, i.e. returning void, true, or a coroutine handle 
from await_suspend.
If the coroutine is not suspended, execution can continue.  If the 
coroutine is suspended, then std::coroutine_handle::resume need to run 
the coroutine on the executor.  Note that in our wrapper, the 
coroutine_handle that the awaitable receives does not have to point 
directly to the parent coroutine.

Highly simplified pseudo-code to demonstrate the concept, obviously not 
even close to correct C++:

   class awaitable_wrapper {

     auto await_suspend(auto handle, auto env) {
       this->env = env;
       this->handle = handle;
       return this->base_awaitable.await_suspend(this->my_coro());
     }

     auto my_coro() {
       boost::capy::run(this->env)(this->handle);
     }

   };


> That domain knowledge is what the author of an
> IoAwaitable provides. You write a "leaf awaitable." This is explained in
> Section 3.3:
> 
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2026/p4172r1.pdf

Constantly having to referencing out-of-band documents to explain a 
library does not speak highly of the library's documentation.

>>>> ..innocent-looking functions like the following do not work correctly:
>>
> 
> This is not quite right. Your `task` coroutine frame is allocated using the
> default allocator. Suboptimal, but it is what you asked for. The launch
> function works fine. The coroutine executes. The coroutine frame is
> deallocated properly. The invariants are preserved.

OK.  This is not clear from the documentation.

As I stated at the beginning of my review, I consider the documentation 
the sole source of truth.  If something works but is not documented to 
work, then the fact that it works is a coincidence of the current 
implementation that could change at any time, not a guarantee.

> ...I am allowed to prefer TooManyCooks' approach, and I am allowed to say
>> so in my review.
>>
> 
> And I would point out that TMC doesn't have this problem, because TMC
> doesn't have the feature. TooManyCooks does not support customizing the
> frame allocator. It uses global ::operator new unconditionally, and the
> guidance is to "link with tcmalloc, mimalloc, or jemalloc instead of the
> default glibc malloc." If you don't have one of these the CMakeLists.txt
> will even warn you.

True, and I'm sure it makes the benchmarks run much faster.  Given the 
restrictions and the brittleness that follow from this feature, I am not 
sure it is worth its cost in real code.  (I'm also not convinced that it 
*isn't*, for that matter.  I am undecided.  But the burden of proof is 
on the side of the new feature, not the status quo)

TMC's advice to just use a better global allocator makes sense, given 
the vast amounts of non-coroutine-related allocations in my code.  But I 
haven't followed it (yet), because I don't have an allocation related 
peformance problem (yet).


>> I lose the ability to get a result back to the coroutine, but I was never
>> going to be able to do anything useful with that result anyway because
>> boost::capy::run_async swallows return values.  If I want to use the
>> result locally instead of returning it, I can do so within the callback.
>>
> 
> I'm getting a little lost because you are first talking about capy::run(),
> then capy::run_async(). The two functions serve different purposes.

Sorry, I didn't include the greater context of my code snippet.

Here is a simplified example in TMC:

   tmc::task<graphics::texture> do_load_texture(std::string path) {
     auto data = co_await filesystem::read_async(path);
     if (!data) {
       log_error("Failed to load texture {}: read error", path);
       co_return graphics::texture{};
     }
     auto img = images::decode_webp(data.value());
     if (!img) {
       log_error("Failed to load texture {}: corrupt webp", path);
       co_return graphics::texture{};
     }
     co_await tmc::resume_on(this->main_thread_executor);
     co_return this->window->load_texture(img.value());
   }

   std::shared_future<graphics::texture> load_texture(
       std::string const &path) {
     return tmc::post_waitable(
       tmc::cpu_executor(),
       this->do_load_texture(path));
   }

The closest equivalent in Capy would look something like this:

   boost::capy::task<> do_load_texture(
       std::string path,
       std::promise<graphics::texture> promise) {
     auto data = co_await filesystem::read_async(path);
     if (!data) {
       log_error("Failed to load texture {}: read error", path);
       co_return graphics::texture{};
     }
     auto img = images::decode_webp(data.value());
     if (!img) {
       log_error("Failed to load texture {}: corrupt webp", path);
       co_return graphics::texture{};
     }
     auto tex = boost::capy::run(this->main_thread_executor)
       ([&img]() {
       co_return this->window->load_texture(img.value());
     }
     promise.set_value(tex);
   }

   std::shared_future<graphics::texture> load_texture(
       std::string const &path) {
     // Caching code path omitted.

     std::promise<graphics::texture> promise;
     auto future = promise.get_future();

     // Here's the run_async call you were looking for.
     return boost::capy::run_async
       (this->pool.get_executor())
       this->do_load_texture(path, std::move(promise)));
     return future;
   }

As you can see, the return value of boost::capy::run is useless to me in 
this particular case because I can't do anything with the return value 
in the outer coroutine that I can't also do inside the inner coroutine.

Then again, writing the equivalent of tmc::post_waitable for Capy 
shouldn't be difficult:

   template<class Ex, class RV> std::future<RV> post_waitable(
        Ex &executor,
        boost::capy::task<RV> task) { // <- uses default allocator
     std::promise<RV> promise;
     auto future = promise.get_future();
     boost::capy::run_async(executor)(
       [promise = std::move(promise), task = std::move(task)]() {
       promise.set_value(co_await task);
     });
     return future;
   }

So, while capy::run's return value isn't needed for my particular case, 
it definitely isn't useless in general.


-- 
Rainer Deyke - [email protected]
_______________________________________________
Boost mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://lists.boost.org/mailman3/lists/boost.lists.boost.org/
Archived at: https://lists.boost.org/archives/list/[email protected]/message/6XJLKBKHHSJ6H76PVGEFWVFDUAC3FTBI/