Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Join 2 (or more) promises with different types #1985

Open
lewisxy opened this issue Apr 11, 2024 · 4 comments
Open

Join 2 (or more) promises with different types #1985

lewisxy opened this issue Apr 11, 2024 · 4 comments

Comments

@lewisxy
Copy link

lewisxy commented Apr 11, 2024

I would like to join 2 (or more) kj::Promise of different type to produce a single promise that fulfills when all promises fulfills and produces a tuple of 2 (or more) elements corresponding to the return value of each promise. So far, there is only exclusiveJoin which fulfills when one of the promise fulfills, and joinPromises, which only operates on the promises that returns the same type. I also noticed that there are similar requests of this feature (#452) before, and it has even been implemented, but never merged into any releases. Are there any plan to include this feature? Thanks

@jclee
Copy link
Collaborator

jclee commented Apr 12, 2024

Not sure of any exact plans, but:

  • There is a long-standing TODO to eventually provide something like that.
  • The code has changed quite a bit since PR Question: Is there a plan to implement "promise.join" method recently? #452, so it's probably non-trivial work to bring that WIP code up to date.
  • Callers would probably prefer the newer joinPromisesFailFast() instead of joinPromises(), to ensure that exceptions are propagated as they happen instead of at the end.
  • If you don't need the fail-fast behavior, one possible workaround is to just set up a chain of .then()s that captures each of the returned values in sequence and then returns the tuple.
  • I guess another possible workaround would be to use kj::OneOf<T1, T2, ...> to define a type that can be used for all the field values, then use that to convert the promises to a single promise type, join them, then cast the array entries back to the correct types afterward. Of course, that's going to be more verbose and probably less efficient than a proper solution.

@kentonv
Copy link
Member

kentonv commented Apr 12, 2024

So, I created #706 long ago, but it crashed GCC and deeply confused MSVC. But maybe it's time to try again with newer versions of these compilers. Or maybe just on the v2 branch where we only really care about clang.

That said, the functionality tends not to be as useful as you'd expect, because there's usually a simple work-around: just await one promise, then the other:

auto result1 = co_await promise1;
auto result2 = co_await promise2;

The only case where this isn't ideal is if you want exceptions thrown by promise2 to cancel the task even if promise1 is still running. In most cases it doesn't matter than much but occasionally it does, and so offering a way to join the promises would help there.

Actually, I kind of wonder if variadic promises still make sense in the age of coroutines. In the old .then() style, the above work-around was uglier (though still not terrible). Also, with .then(), it made sense that the multiple results would just be passed as multiple arguments to the callback, so the existence of tuples was entirely hidden. With coroutines I suppose co_awaiting a multi-value promise would return a tuple which you then have to destructure... maybe that's fine. (But I think we want to avoid the need to use kj::get<n>(tuple) syntax at all costs... it's just so ugly...)

@kentonv
Copy link
Member

kentonv commented Apr 12, 2024

In conclusion... this doesn't feel like a pressing need, especially with the advent of coroutines. So I think it's unlikely I'll pick this up anytime soon. If someone really wants to take the old PR and try to update it though, go ahead.

@lewisxy
Copy link
Author

lewisxy commented Apr 13, 2024

Unfortunately, the project I am working on stuck at C++17 for now, so no coroutine. It isn't super performance demanding right now, so my workaround is to chain or nest .then(...). My main complaint is that is looks kinda ugly and isn't easy to maintain if there are too many promises depending on each other.

I guess another possible workaround would be to use kj::OneOf<T1, T2, ...> to define a type that can be used for all the field values, then use that to convert the promises to a single promise type...

I thought about this approach before, with std::variant though, but the idea is the same. While it can work, it feels like a hack, and I wouldn't want to maintain code like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants