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

Why doesn't dash::copy support GlobPtr as input? #246

Open
BenjaProg opened this issue Jan 20, 2017 · 9 comments
Open

Why doesn't dash::copy support GlobPtr as input? #246

BenjaProg opened this issue Jan 20, 2017 · 9 comments

Comments

@BenjaProg
Copy link
Member

Hey, is there a reason why dash::copy doesn't accept GlobPtr as in/output?
I stumbled about this as i wanted to copy to a globmem allocated with memalloc on a remote unit.
it's just in the comments right now and I try to solve the problem another way, but I am still curious why ;-)

@fuchsto
Copy link
Member

fuchsto commented Jan 20, 2017

dash::copy is an algorithm interface, that is: it has several implementations depending on the direction (local-global, global-local, global-global) with adapter wrappers for different parameter types.
GlobPtr ranges should be supported and, at least for some variants, already are.
Apparently an adapter specialization for GlobPtr is missing for your use case. I will add it.

@BenjaProg BenjaProg changed the title Why doens't dash::copy support GlobPtr as input? Why doesn't dash::copy support GlobPtr as input? Jan 21, 2017
@fmoessbauer
Copy link
Member

As discussed in #325 a GlobPtr does not respect the structure of values in memory. Passing a GlobPtr to dash::copy should not be allowed. Taken again the std::list example, this demonstrates why: Passing a pointer to a list element to an STL algorithm does not work because the data is not contiguous in memory. There you need full blown iterators.

Keeping that in mind, I guess we can close this issue.

@fuchsto
Copy link
Member

fuchsto commented Mar 17, 2017

@fmoessbauer That's right in principle, but std::copy accepts pointers and so should we.

@fuchsto fuchsto reopened this Mar 17, 2017
@devreal
Copy link
Member

devreal commented Mar 17, 2017

We could annotate GlobPtr type to signal whether it points to contiguous memory (i.e., it points to a dash::Matrix or dash::Array) and at runtime error out if the user tries to copy more than one element and the memory region pointed to is not contiguous. This might be more restrictive than necessary but prevents from running total havoc in the transport layer.

@fuchsto
Copy link
Member

fuchsto commented Mar 17, 2017

I think it should not. GlobPtr is a dull representation of an address. Or thinking the other way around: In which situation would application programmers use dash::copy or std::copy (which is explicitly supported!) on a GlobPtr? To avoid overhead from validation and index calculations.
Also, GlobPtr has no concept of containers (other than GlobIter types) so its implementation has no means to reflect container memory, like contiguous data. Hey, perhaps copying across the iteration space it's exactly what the application developer wants to do? It's not the concern of DASH algorithms or GlobPtr.

@fuchsto
Copy link
Member

fuchsto commented Mar 28, 2017

Update: GlobPtr<T> now provides robust global random access pointer semantics for any memory space so dash::copy can be safely extended to support global pointers now.

@BenjaProg
Copy link
Member Author

cool, nice improvement! :-)

@devreal
Copy link
Member

devreal commented Mar 7, 2018

Is anyone working on this? @tuxamito ran into this problem today. @rkowalewski is that part of your upcoming PR?

@tuxamito
Copy link
Contributor

tuxamito commented Mar 8, 2018

I totally need this feature to be able to access arrays pointed by GlobPtr. Accessing the elements one by one is terribly slow :(

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

8 participants