----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5389/#review8424 -----------------------------------------------------------
Ship it! I don't see anything here that raises alarms. I am not, however, very familiar with JNI. Some comments: 1) The JNI_OnLoad function seems to do things that I expected Swig to do for you (declare exceptions). 2) I'll be interested to know if the Java-side codec for message bodies is performant. 3) I wonder if the lack of nested list/map support in the headers is going to be a problem for anybody. - Ted Ross On June 18, 2012, 4:30 p.m., rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5389/ > ----------------------------------------------------------- > > (Updated June 18, 2012, 4:30 p.m.) > > > Review request for qpid, Gordon Sim, Kenneth Giusti, and Ted Ross. > > > Description > ------- > > The binding consists of several pieces. > > 1. The swig interface files which defines type maps, exception mapping etc. > 2. C++ and Java helper functions for type maps (currently resides in the swig > files itself). --> I would like to put this in proper source files going > forward. > 3. The Java implementation. > > This review is only for items 1 & 2. > > The following are some notes that should help with the review. > > 1. Class and method definitions are pre loaded for performance. I'm using the > JNI_OnLoad method for this. > 2. Message body is encoded and decoded on the Java side. > 3. Direct byte buffers are used for avoiding an extra copy across the JNI > boundary. This allows Java and C++ to access memory defined by each other. > 4. Only primitives and strings (and UUID's) will be supportted for > application headers. > 5. The underlying VariantMap (for application headers) will be wrapped by a > Java Map and access is on demand. > 6. C++ objects are deleted as soon as they are not required. Ex > connection/session/receiver/sender closed. Failing which Java wrapper objects > will cleanup the corresponding C++ object in it's finalizer when it gets > garbage collected. > 7. c++ Message objects (for messages received) will be cleaned up when the > corresponding Java mesage object goes out of scope. If this becomes a problem > we might have to go for a more agressive strategy. > 8. The Java wrapper object keeps the pointer to a c++ object as a long. All > JNI calls will pass this which will be used to refer to the C++ object when > calling the methods. > > > This addresses bug QPID-4027. > https://issues.apache.org/jira/browse/QPID-4027 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/qpid/CMakeLists.txt > 1351358 > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/qpid/java/.gitignore > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/qpid/java/CMakeLists.txt > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/qpid/java/LICENSE > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/qpid/java/java.i > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/swig_java_cpp_helper.i > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/swig_java_helper.i > PRE-CREATION > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/bindings/swig_java_typemaps.i > PRE-CREATION > > Diff: https://reviews.apache.org/r/5389/diff/ > > > Testing > ------- > > I have tested this manually. It would be good if I could add some unit > testing to some of the c++ code. Currently they are embedded in the swig > files, so I wasn't sure how this would work. > > > Thanks, > > rajith attapattu > >
