Memory leaks? - Githubissues

222464 commented 8 years ago

Hello,

This may come from a place of ignorance, but isn't all the new without delete usage leaking tons and tons of memory? I don't see any smart-pointers anywhere. Also I thought new/delete was obsolete?

marty1885 commented 8 years ago

I'm not the maintainer, but could you provide some code that deoms memory leak so I can have a look on what's happening?

I'm curious on the Smart Pointer topic too. But on the opposite side. Smart Pointers are slow to compile and might give bad performance comparing to regular pointers. Basically any STL object is slower than the C stuff (vector vs arrays, smart pointers vs pointers, etc...). (I ran 30% faster by replacing std::stack by an array on my Ray Tracer's BVH Accelerator.) It's just my point of view tho. Do anyone have other opinions on this topic? I'm really curious about it.

hughperkins commented 8 years ago

@222464

Can you give an example of a new you see that doesnt have a matching delete?

222464 commented 8 years ago

I don't remember which one I thought I saw not having the matching deletes. Perhaps they were there and I was mistaken. However, why use new/delete at all? It's not good practice for modern C++.

@marty1885 When used properly, smart pointers can be exactly as fast as regular pointers. Also, it is not true that STL objects are slower than C stuff. std::vector, for instance, is just as fast as a heap-allocated primitive array. Stack allocated arrays? std::array has you covered. I actually wrote a raytracer too :) I used to do a lot of 3D programming.

hughperkins commented 8 years ago

Hi 222464,

Thank you for taking an interest in DeepCL, and thank you for the feedback on using smart pointers. As you can imagine, in any project there are many tiny decisions, in design, which have to be made at each stage. CUDA vs OpenCL. Logistic regression vs deep learning. And so on. Using smart pointers vs primitive float arrays is one of many such decisions. I understand it's different from how you would design the project yourself. I guess if you are looking at the code, you are contemplating making some contributions to DeepCL? And I assume that the projects you have worked on have used smart pointers, and it looks a bit ... backwards? ... to you to not use them?

Please bear in mind that in DeepCL, the major painpoint is not writing C code, which is relatively straightforward, I think, but: writing OpenCL kernels, and getting them to run fast. This is really hard and a major pain point. If you can make fast convolutional kernels in OpenCL, well, there are tons of conferences that might be interested in papers and so on on this topic :-)

222464 commented 8 years ago

Hello,

I agree that writing the OpenCL kernels is where the thought goes in (I do a lot of GPGPU programming as well), the C++ side is mainly boilerplate to call the kernels.

I was looking into using DeepCL as part of a pipeline in another algorithm, as well as using it to compare to the techniques I am working on. Most C++ projects I use myself use modern C++ (11+), so I was a bit surprised that the (pretty much only) DL library for OpenCL is written in an older style.

Perhaps I can make a fork at some point with modern C++ updates :)

hughperkins commented 8 years ago

Well... I used c++11 originally, but came across two issues:

first is that python 2.7, on windows uses msvc2008, which I'm not sure handles c++11?
second is that some gcc users are still on 4.4, again I'm not sure handles c++11?

For the first issue, I've split the build now into two parts:

python cython code, builds on msvc2008, msvc2010, depending on python version
main c++ code, builds on msvc2010 currently (in windows build)

Therefore, it's not impossible the first issue could be resolved now, though changing from msvc10 to some more recent msvc version would need careful testing. Windows 7 should continue to be supported.

For the second issue, I'm not sure if there is an easy solution?

On the whole, if I can choose larger userbase, or slightly more pain for me, I choose slightly more pain for me :-)

hughperkins commented 8 years ago

On the whole, if you want to migrate to some more recent versoin of msvc, and handle testing in all the various combinations of builds, then I might be approximatley open to merging that. You'd need to demonstrate clearly that everything continues to work on :

Windows 7 32-bit, python 2.7
Windows 7 64-bit, python 2.7
Windows 7 32-bit, python 3.4
Windows 7 64-bit, python 3.4
Mac OS X, python 2.7 (with SIP on)
Mac OS X, python 3.4 (with SIP on)
Ubuntu ... hmmm... 14.04 is still the standard on ec2 actually, so 14.04, 32-bit, python 2.7
Ubuntu 14.04 64-bit, python 2.7
Ubuntu 14.04 32-bit, python 3.4
Ubuntu 14.04 64-bit, python 3.4

I have jenkins builds that can handle a lot of the linux stuff. I have travis builds for mac (but no tests). Windows testing is kind of annoying, since I dont have a Windows box, and have to remote in to ec2 boxes and stuff...

hughperkins commented 8 years ago

(there are a bunch of jenkins scripts for windows and linux at: https://github.com/hughperkins/DeepCL/tree/master/jenkins The entrypoints are approximately described in https://github.com/hughperkins/DeepCL/blob/master/jenkins/jobs.yaml travis is at https://travis-ci.org/hughperkins/DeepCL )