higan-emu / libco

libco is a cooperative multithreading library written in C89.
Other
125 stars 25 forks source link

User data/closure support #16

Closed moon-chilled closed 4 years ago

moon-chilled commented 4 years ago

Originally from https://github.com/higan-emu/higan/issues/51. I can contribute a version for amd64, sjlj, ucontext, and probably x86 too. But someone would have to handle arm, ppc, winfiber.

Additional consideration: there should probably also be a callback to destroy the userdata. Does that clutter the API too much? Should there be co_create and co_create_ext, where the latter accepts userdata+destructor?

Alcaro commented 4 years ago

I personally would like to see userdata, but it would be hard to add without breaking the API, or adding a new function with a silly name like co_create_ex or co_create_2. Unlike most APIs without userdata, a libco callee is only entered once, so it's not hard to work around (though it needs mutexes or global variables).

I vote no to userdata destructor. Most of the time, userdata should be 'this' on co_create's caller, or just null; in the last few cases, better to tell co_delete's caller to take care of that (possibly via some suitable C++ wrapper with a destructor). One of libco's main goals is simplicity; increasing the API size to potentially remove one line from co_delete's caller doesn't quite feel simple to me.

moon-chilled commented 4 years ago

hard to add without breaking the API

How important is that? Are there any major consumers beyond higan? I considered mentioning ABI compatibility as a benefit of co_create_ext, but decided it didn't matter enough.

Most of the time, userdata should be 'this' on co_create's caller

That makes a big assumption: that the consumer is c++. In c, the state will almost always be an allocated struct pointer, which you almost always just want to free. Making the caller do cleanup isn't just one line of code; it means the caller also has to keep around a copy of the user data pointer, which means putting it in a struct. That further means a coroutine handle doesn't fit in a register, which can have performance implications if you're passing it around a lot.

merryhime commented 4 years ago

Are there any major consumers beyond higan?

Canonical and Fluent Bit are two major consumers.

Alcaro commented 4 years ago

Canonical and Fluent Bit are two major consumers.

And the libretro team (though they have a fork - I think they added support for a few more oddball platforms, like PSP. We should merge their changes, or set a clear goal statement that explains why we're not merging.)

Making the caller do cleanup isn't just one line of code; it means the caller also has to keep around a copy of the user data pointer, which means putting it in a struct

which caller already does under most plausible program architectures I can think of. For the others, userdata can contain a copy of the coro handle, callee can clean up before telling caller to delete the routine, or multiple or all of the above.

I can think of a few program architectures where none of the above is true, but unless I'm missing something massive, they're too rare to design anything around. If you really need it, you can create a libco wrapper where my_co_create allocates a struct containing the real cothread_t, and the destructor.

That further means a coroutine handle doesn't fit in a register, which can have performance implications if you're passing it around a lot.

You want to pass around a pointer to a deletion function often enough that it's a performance concern? Without actually calling it (free() and co_delete() cost more than passing a parameter thousands of times)?

Are you sure?

Screwtapello commented 4 years ago

I feel like libco's strengths are simplicity and portability, and (convenient though it would be) adding user data/closure support doesn't improve either of those.

I'm going to mark this WONTFIX.

bitraft commented 3 years ago

adding user data/closure made libco more easy to use with simple code, and not hurt portability.

markand commented 1 year ago

I was also thinking of using this library because it's clean and simple but having to use global variables is a real blocker. Given that there are no real release engineering/semantic versioning why not break the API for something really important?

Or at least, I'd be glad to know how do you manage to avoid globals and still have reentrant programs?