Closed GoogleCodeExporter closed 9 years ago
Ryan
I want to thank you and Steven for taking a stab at tidying this up. As we move
forward I think this will help us move Aparapi forward and potentially add
other GPU enabled back ends such as the HSA foundation's HSA architecture.
http://hsafoundation.com/
Let me sync with the branch
http://code.google.com/p/aparapi/source/browse/#svn%2Fbranches%2Faparapi-issues-
71-44
and start taking a look. I know there is a lot of code changed here so this
may take a while.
Original comment by frost.g...@gmail.com
on 29 Oct 2012 at 7:20
Thanks Gary
Original comment by ryan.lam...@gmail.com
on 29 Oct 2012 at 9:15
Would you mind if I ask the global mailing list to take on this task as well?
We have made some additional C++ JNI changes, but are still tracking down the
JTP bug.
Original comment by ryan.lam...@gmail.com
on 19 Nov 2012 at 8:11
The JTP bug has been resolved.
Original comment by ryan.lam...@gmail.com
on 21 Nov 2012 at 2:54
Hi Gary, just a ping on this issue, since it has been a month since the code
review request and the trunk changes are really adding up.
Original comment by ryan.lam...@gmail.com
on 26 Nov 2012 at 10:25
Sorry Ryan. I am on vacation (exhausted from todays Paragliding lessons ;) ) I
will take a look at this when I get back (assuming I survive my lessons
tomorrow and Wednesday!).
I do apologize.
Gary
Original comment by frost.g...@gmail.com
on 26 Nov 2012 at 10:56
No problem Gary, please enjoy your vacation.
On Nov 26, 2012 2:56 PM, <aparapi@googlecode.com> wrote:
Original comment by ryan.lam...@gmail.com
on 27 Nov 2012 at 5:21
Hi Gary, I wanted to know what the status on the code review is. Right now
it's blocking me on making progress on implementing multidimensional arrays.
Original comment by Steven.L...@gmail.com
on 4 Dec 2012 at 7:27
Steven,
So I just synced with the latest code and took a quick look. Of course my
first test was to compile. Compilation failed for me on Linux. My guess is
this will compile on Windows because my errors are due to file name case
inconsistencies. For example file clHelper.h is included as CLHelper.h and is
failing. My guess is this will succeed on Windows (is Mac OSX tolerant of
this?).
So my first suggestion is that we check code to ensure it is consistent (file
name case with include case).
Gary
Original comment by frost.g...@gmail.com
on 4 Dec 2012 at 8:32
I checked in some edits (r887) to allow header files to be located. Now I have
a bunch of warnings (string to char*) which may be OK. But can't get past these
compile errors from gcc.
src/cpp/invoke/opencljni.cpp: In instantiation of ‘void putPrimative(JNIEnv*,
cl_kernel, jobject, jint) [with jT = int; cl_T = int; JNIEnv = JNIEnv_;
cl_kernel = _cl_kernel*; jobject = _jobject*; jint = int]’:
src/cpp/invoke/opencljni.cpp:211:64: required from here
src/cpp/invoke/opencljni.cpp:151:7: error: cannot pass objects of
non-trivially-copyable type ‘std::string {aka class
std::basic_string<char>}’ through ‘...’
and
src/cpp/invoke/OpenCLArgDescriptor.h:15:22: error: extra qualification
‘OpenCLArgDescriptor::’ on member ‘getMemInstance’ [-fpermissive]
In file included from src/cpp/invoke/OpenCLArgDescriptor.h:4:0,
from src/cpp/invoke/OpenCLArgDescriptor.cpp:1:
Original comment by frost.g...@gmail.com
on 4 Dec 2012 at 8:58
Gary,
Apologies for the failures on Linux. We have thus far only tested on Windows 7
64-bit (VS 2010) and OS X 10.7. We will work on getting the branch code into a
Linux environment and testing it there.
Were you able to build in Windows 7 or OS X?
A couple of quick questions:
- What Linux are you using?
- What GCC are you using?
We've seen a number of inconsistencies between GCC versions that have caused us
grief.
Original comment by ryan.lam...@gmail.com
on 4 Dec 2012 at 10:31
No problem. I just happened to be on a Linux machine waiting for a
build to finish. So I tried compiling there.
I was using Ubuntu 64 bit 12.10 I think
gcc version on this machine was 4.7.2
BTW I checked my header changes in, and I think the other two issues
above can be solved with an appropriate cast. But didn't want to
screw things up :)
Yes I did get it to build and run on Windows and started looking
through the code. We seemed to have lost maybe 5% performance
somewhere which is odd... can you guys confirm this? I have seen it on
two machines.... One with APU and one with GPU.
I will look deeper at the code.
I will re-iterate my concern about using STL. Again, I will note that
OpenJDK JVM does not use it and I always worry about
portability/consistency. I want to try to re-factor some of this code
so it can be used inside the JVM (Sumatra) and I am a little reticent
to introduce dependencies which would not be compatible with the
OpenJDK code base.
Gary
Original comment by frost.g...@gmail.com
on 4 Dec 2012 at 11:05
It is interesting over the years hearing people's concerns about STL not being
cross-platform. That may have been true prior to the late-1990's, but I cannot
think of a legitimate reason today why a C++ developer would not use the STL. I
think it is simply a historical mindset, exactly how "Java is slow" is
widespread because the last time people used it was 1999. I'm guessing OpenJDK
does not use STL because the code base was started in the early 1990's. What we
do see though is people avoiding STL and then writing all of their own
re-implementations of STL, which has all kinds of potential drawbacks. I think
this is an important discussion, but definitely one which should have the
OpenJDK folks weigh-in(if that is possible)...maybe there are actual technical
reasons for not using STL in OpenJDK, maybe not?
Anyway, I'm very interested in the performance degradation. Do you have a
specific test with timing that you would like us to verify?
Original comment by ryan.lam...@gmail.com
on 5 Dec 2012 at 12:38
Of course, we could always use the BoostLibraries instead :)
Original comment by ryan.lam...@gmail.com
on 5 Dec 2012 at 12:41
:)
I was trying to track down a reason why OpenJDK seems to steer clear of STL. I
saw a few comments regarding not being able to control allocations... but I
really can't say that there is a concrete reason. I am just aware that it is
not used. Lots of home build collections, maps and sets..
Original comment by frost.g...@gmail.com
on 5 Dec 2012 at 1:43
Yes, unless I am mistaken, it is my understanding that since ~2003 pretty much
all of the major compilers have standardized on the C++ Standard Library and
Standard Library Template.
Original comment by ryan.lam...@gmail.com
on 5 Dec 2012 at 2:31
Ok, I went through and tried compiling the c++ code on linux. I fixed the
errors, so it compiles now, and I eliminated several of the warnings. It turns
out g++ takes const more seriously than visual studio does. The only warnings
I have now are related to printf statements getting arguments of different
sizes than it expects.
Original comment by Steven.L...@gmail.com
on 5 Dec 2012 at 6:46
Thanks Steven.
I will dive back in.
Gary
Original comment by frost.g...@gmail.com
on 5 Dec 2012 at 7:19
Steven
I have reviewed the cpp code. Still need to look at the java. I like the
cleanup that you have done, and want to thank you for it.
I only have one request. Can we be consistent with file name case. I note we
have some headers CLxxxx.h and others clxxxx.h. I have built a CDT project
for building/navigating this new structure from within Eclipse CDT. Let me know
if you would find this useful, I will check the profile files into svn.
I hope to get around to the Java changes soon. I do apologize for my
tardiness. Lots of spinning plates at the moment.
One more question, what is the strategy for the merge. I guess we need to diff
the current trunk with the version you copied for this branch and use this
subset to update your branch (we have some small patches since the fork, I am
sure).
Gary
Original comment by frost.g...@gmail.com
on 6 Dec 2012 at 9:43
Gary,
Apologies for the filename casing inconsistencies. I will speak with Steven
about getting those corrected. Please feel free to refactor as necessary, we
are hoping to full two-way development efforts in this branch.
Using Eclipse CDT would be highly desirable for me, at least and I would
welcome your check-ins of that configuration for Aparapi. It will be
interesting seeing CDT working cross-platform.
As for Trunk merging, what I imagine we'll have to do is the following:
- Take inventory of all changes to the Trunk since this branch was cut
- Merge in all Trunk changes to the branch
- Merge Branch into Trunk...which may just be a complete overwrite if Branch
has all Trunk changes applied
I'm still very interested in tracking down the performance issues you are
seeing. Are they in both JTP and GPU mode? Are they pre or post OpenCL
generation? Do you have a specific test you were using that we can also use to
track this together?
Original comment by ryan.lam...@gmail.com
on 8 Dec 2012 at 1:41
Gary,
That's a good point. File name conventions in c++ are iffy at best. I changed
the file names to reflect the conventions in the java code, so now they're
consistent across the project.
Steven
Original comment by Steven.L...@gmail.com
on 10 Dec 2012 at 7:15
Gary,
All C++ files and classes have been renamed to be consistent with Java.
Original comment by ryan.lam...@gmail.com
on 11 Dec 2012 at 5:44
Thanks Guys. Still have not got back into Java code review. I hope to do so
over the next day or so.
Original comment by frost.g...@gmail.com
on 11 Dec 2012 at 6:19
Gary,
I've been investigating the 5% decrease in performance you mentioned. I hope to
write an actual timing test soon, but I've not yet decided how that should be
conducted, any suggestions are welcome.
Here is what I've seen so far on two sample projects:
Mandel FPS:
Trunk is equal to Branch
Life FPS:
Trunk is 5%-8% slower than Branch
Original comment by ryan.lam...@gmail.com
on 14 Dec 2012 at 11:30
Original comment by ryan.lam...@gmail.com
on 20 Apr 2013 at 12:37
Original issue reported on code.google.com by
ryan.lam...@gmail.com
on 29 Oct 2012 at 6:23