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

smart pointer KernelArg #45

Open
zeratax opened this issue Dec 4, 2019 · 12 comments
Open

smart pointer KernelArg #45

zeratax opened this issue Dec 4, 2019 · 12 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects

Comments

@zeratax
Copy link
Owner

zeratax commented Dec 4, 2019

es ist vielleicht cooler KernelArg mit smart pointer zu initalisieren, aber ich weiß nicht ob man implizit pointer zu smart pointern machen kann, e.g. &a wenn a ein int ist, also für die valueargs. Frag mich auch ob smart pointer cooler für jni oder schlimmer wären.

@zeratax zeratax added the enhancement New feature or request label Dec 4, 2019
@zeratax
Copy link
Owner Author

zeratax commented Dec 4, 2019

Vielleicht so

KernelArg(std::shared_ptr host_data, bool download = false, bool copy = true, bool upload = true);

// und dann für alle basic types (ValueArg) 
KernelArg(int *host_data, bool download = false) : KernelArg{std::make_shared(host_data), download, false, false} {};
KernelArg(const int &host_data) : KernelArg{std::make_shared(host_data), false, false, false} {};

vielleicht kann man für basic types das auch templaten auch mit ner shared library, weiß nicht?

Edit:
Weiß nicht ob die defaults für non const into gut sind.
copy braucht man auch nicht, oder? eig ist nur die frage ob man runterlädt bei pods?
denke eig wenn man nicht runterladen will, sollte man const nehmen, aber kann mir vorstellen, dass man trotzdem einfach

int a{5};
KernelArg{&a};

macht für eine konstantes argument, weil der host code das als mutable braucht??

@LukasSiefke
Copy link
Collaborator

Für die JNI wärs glaube ich ganz cool
Dann braucht man die eine Wrapper-Klasse nicht

@zeratax
Copy link
Owner Author

zeratax commented Dec 5, 2019

für fundamental/basic types sollte das hier helfen:
https://en.cppreference.com/w/cpp/types/enable_if
https://en.cppreference.com/w/cpp/types/is_fundamental

aber make_shared von einem stack object klingt halt sehr falsch?
vielleicht braucht KernelArg zwei member variablen einmal für nen shared pointer und void* = nullptr, dass dann für stack objekte benutzt wird und nur benutzt wird wenn es =! nullptr ist?

Edit: eig nur fundamental types, oder auch alle pod?
https://en.cppreference.com/w/cpp/types/is_pod

@zeratax
Copy link
Owner Author

zeratax commented Dec 5, 2019

sehr nervig, dass man dann immer noch size übergeben muss, das fehlt da auch noch. eig ziemlich sicher, dass der shared pointer das speichert aber naja

@zeratax
Copy link
Owner Author

zeratax commented Dec 5, 2019

oh noch eine sache, zur zeit wird ja pro argument hochgeladen, aber man könnte auch alle auf einmal hochladen, siehe: https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__MEM.html#group__CUDA__MEM_1g27f885b30c34cc20a663a671dbf6fc27
würde den overhead noch etwas veringern.

@zeratax
Copy link
Owner Author

zeratax commented Dec 5, 2019

oh noch etwas. pointer pointer ist eig immer ne schlechte idee zum hochladen und sollte immer geflattened werden.
also wenn der smart pointer auf nen pointer zeigt das ist schlecht.
float[][] kann man nicht zum gpu trivial hochladen.

obwohl es auch extrem umständlich ist mit shared_ptr<float**> zu arbeiten, passiert also wahrscheinlich nie.

@zeratax
Copy link
Owner Author

zeratax commented Dec 6, 2019

hier so funktionierende templates: https://wandbox.org/permlink/Ij0mH10oaCj6GrQ8

was mich hier stört ist, dass das hier schwer bis garnicht möglich ist mit vektoren auf der host seite zu arbeiten und die dann als array auf dem device zu benutzen:

std::vector<pixel> image;
image.resize(size);
KernelArgs{image.data(), size};

und den vector data zu einen smart pointer machen ist auch kein guter style, da dann vector und smart pointer den destruktor auf den selben pointer anwenden wollen.

Edit:
das hier erledigt eig gleich 2 probleme, falls man das in shared libraries benutzen darf, was ich bezweifle:
https://wandbox.org/permlink/QDd0QtKbTb9Uucxv

template<typename Iterator>
std::string KernelArg(Iterator begin, Iterator end){
    size_t size = std::distance(begin, end) * sizeof(*begin);
    return std::string {"iterator, size: "} + std::to_string(size);
}

so hier geht es auch:

std::string KernelArg(void* begin, void* end) {
    intptr_t size = reinterpret_cast<intptr_t>(end) - reinterpret_cast<intptr_t>(begin);
    return std::string {"iterator, size: "} + std::to_string(size);
}

aber dann muss man das so aufrufen:

 KernelArg(image_vector.data(), image_vector.data() + size); // kann man auch gleich void*, size
 KernelArg(&(*image_vector.begin()), &(*image_vector.end())); // ehm...

Edit Edit:
denke immer, dass man doch void* braucht auch wenn der rest hier trotzdem hilfreich ist.
vielleicht sollte man dann bei dem void* irg wie sowas machen?

template<typename T>
std::string KernelArg(T* test) {
    if(!std::is_same<typename std::remove_pointer<typename std::remove_pointer<T*>::type>::type, typename std::remove_pointer<T*>::type>::value)
        throw 1;
    return "void pointer";
}

template bool KernelArg<void>(void* test); // explizite Instanziierung für void*

float* float_ptr;
float** float_ptr_ptr;
KernelArg(float_ptr);
KernelArg(float_ptr_ptr); // wirft exception

static_assert denke ich macht keinen sinn für shared library, vielleicht assert, aber weiß nicht was einfacher für jni ist.

die range angeben gefällt mir syntaktisch aber am besten

@visualJames
Copy link
Collaborator

Die ursprüngliche Problematik ist, dass wir SIGSEGV, also Segmentationfaults bekommen und unser Programm abstürzt. Wir wollen erstmal zumindest wissen, wo und warum ein SIGSEGV aufgetreten ist. Wir könnten diese Signals, die in http://www.cplusplus.com/reference/csignal/ beschrieben sind, handeln und wie bei der checkCudaErrors Fkt, diese dann abfangen und Zeile, File und Beschreibung zurückgeben.

@zeratax
Copy link
Owner Author

zeratax commented Dec 10, 2019

An sich syntaktisch am klarsten und sichersten wäre eig ein move?

const size_t SIZE = 1024;
auto host_array = make_unique<float[]>(SIZE);

std::vector<KernelArg> args;
auto device_array = KernelArg{std::move(host_array)};
args.pushback(device_array);
args.emplace_back(KernelArg{SIZE});

kernel.launch(args);

std::unique_ptr<float*> host_array = device_array.download();

mit KernelArg header so:

void KernelArg::KernelArg(std::unique_ptr hdata, bool download = false, bool copy = true, bool upload = true);
std::unique_ptr KernelArg::download();

muss man halt sehr explizit runterladen und man kann die variable nicht mehr weiterbenutzen...

ob man für vektoren vielleicht mit std::any das safe machen könnte?

@LukasSiefke
Copy link
Collaborator

Also fürs JNI wäre ein vollständiges Kopieren noch praktischer, da das Java-Array sowieso einmal in ein neues C-Array kopiert werden muss. Dann könnte man auch die alten Variablen weiter benutzen...

@zeratax
Copy link
Owner Author

zeratax commented Dec 10, 2019

std::copy geht auch mit std::unique_ptr denke ich?
ja ich probier das vielleicht mal, schreibe glaube ich erstmal bastian an

@zeratax zeratax added this to To do in v1.0.0 Dec 12, 2019
@zeratax
Copy link
Owner Author

zeratax commented Dec 13, 2019

Hier nochmal alles als juypter notebook zusammengefasst:
https://github.com/ZerataX/notebooks/blob/master/notebooks/KernelArgs.ipynb

@zeratax zeratax added the help wanted Extra attention is needed label Dec 14, 2019
@zeratax zeratax moved this from To do to High Priority in v1.0.0 Dec 15, 2019
@zeratax zeratax mentioned this issue Mar 28, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
v1.0.0
  
High Priority
Development

No branches or pull requests

3 participants