libunifex icon indicating copy to clipboard operation
libunifex copied to clipboard

[WIP] P2300 bulk implementation to libunifex

Open RishabhRD opened this issue 2 years ago • 6 comments

This PR intends to add P2300 std::execution::bulk implementation to libunifex under name unifex::bulk.

RishabhRD avatar Sep 30 '21 19:09 RishabhRD

The PR title says WIP, which I read as "work in progress", but there's no activity since September 30; do you intend to iterate, or are you waiting for a review?

ispeters avatar Nov 14 '21 02:11 ispeters

@ispeters Apologies, I was unable to pay attention to the PR from a long time because I am going through my undergraduate job interviews, so I got busy in preparation and interview stuffs. It would take 10-12 more days for interviews to be over. I would start working on this PR after that.

Currently the PR just fulfills the spec as described in P2300, however unifex::bulk_transform and other related modules are doing some extra work that I am not sure of and I need to read that. Based on that I would progress on the current PR once I start working.

If someone is doing the same work feel free to close this PR in favor of that as I still need 10-12 days to start working and project progress is way more important.

Thanks

RishabhRD avatar Nov 14 '21 18:11 RishabhRD

Oh, goodness, @RishabhRD, no need to apologize, I was just checking in. If you intend to iterate when you have time, we'll wait for you. Thanks for the update, and good luck with those interviews!

ispeters avatar Nov 14 '21 20:11 ispeters

@ispeters I need to discuss some of implementation details from unifex::bulk_transform and unifex::bulk_schedule.

Both of the modules use unifex::set_next for every index in shape space. This set_next is not proposed by P2300. Currently my implementation does the things prescribed in P2300. Should I add set_next thing to my implementation.

Behavioral difference between set_next implementation and P2300 proposal is:

  • bulk_transform can accept sender created by bulk_schedule and accepts an invocable. bulk_schedule calls unifex::set_next for every index in shape space. bulk_transform's internal receiver implements set_next that invoke the given invocable with index passed by bulk_schedule to unifex::set_next and then it in turn calls unifex::set_next with the result of invocation of given function. Function Signature of set next: unifex::set_next(Receiver, index)

  • P2300 proposes that execution::bulk accepts a sender, shape and a invocable. When set_value would be called, set_value implementation of bulk's internal receiver should invoke given invocable for all index in given shape space along with the value passed to set_value function and then call execution::set_value with those value passed.

In the above implementation the invocable has nothing to do with values passed in set_value. So, they are quite different. So, should I choose set_next direction?

RishabhRD avatar Nov 22 '21 15:11 RishabhRD

@LeeHowes, I think you're the expert on the bulk_ methods; do you have opinions on @RishabhRD's questions?

ispeters avatar Feb 09 '22 02:02 ispeters

Yes, I think matching the p2300 design makes most sense at this point, even with all of its flaws. P2300 looks like it is removing the default looping implementation of bulk, so really the appropriate thing to do would be to customise on the scheduler so that there is a bulk implementation for each of the built in execution contexts.

P2300 uses the clunky method of propagating a scheduler with the senders, I don't think that's implemented in libunifex, though. So this would probably mean also implementing the get_completion_scheduler functionality as well.

LeeHowes avatar Feb 09 '22 03:02 LeeHowes