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

RFC: Solve streaming terminal reducer signature inefficiencies #170

Open
devreal opened this issue Oct 18, 2021 · 2 comments
Open

RFC: Solve streaming terminal reducer signature inefficiencies #170

devreal opened this issue Oct 18, 2021 · 2 comments
Assignees

Comments

@devreal
Copy link
Contributor

devreal commented Oct 18, 2021

While working on the MRA benchmark, I noticed that the design of the streaming terminals forces us to make two copies for each reduction: the new element to be reduced and the copy of the result into the old element. That seems way too inefficient...

The current signature looks like this:

T&& reduce_fn(T&& t, U&& u);

Notice that reduce_fn takes ownership of both arguments and returns ownership of the result (which might be t, as in MRA). This might be ok for simple types (integers, doubles) but is terrible for structures, esp. PODs, as it forces us to make a copy before invocation in order to keep the copy that we're tracking intact (after all, we might want to pass it to other tasks too). See https://github.com/TESSEorg/ttg/blob/master/ttg/ttg/parsec/ttg.h#L1349 Note that since reduce_fn is wrapped by std::function the compiler cannot elide the return value copy.

I'd like to propose changing the signature to so that no copies in the backend are required:

void reduce_fn(T& t, const U& u);

That way, t can be updated with u but u stays constant. If that involves copying u so be it, but we shouldn't be forced to make a copy preemptively in the backend.

@evaleev
Copy link
Contributor

evaleev commented Oct 20, 2021

@devreal sounds reasonable ... I'd imagine some corner cases where you know u can and should be scavenged, but it's not clear how to overcome the single invocation signature forced upon us by std::function.

@devreal
Copy link
Contributor Author

devreal commented Oct 29, 2021

The first part is solved, we need to work on the T/U dual-type signature.

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

No branches or pull requests

3 participants