Re: [cp-patches] RFC: new VM interface for Socket impls

[Roman Kennke]

Hi there,

I have put together a new proposal for the VM interface for
Plain(Datagram)SocketImpl. I incorporated most of Marks suggestions.
Also I introduced an optimization for
VMPlainDatagramSocketImpl.receive() which turns out to be significantly
faster than before because it avoids copying stuff back and forth from
java to native and vice versa.

Am Mittwoch, den 07.12.2005, 13:35 +0100 schrieb Mark Wielaard:
> On Fri, 2005-11-11 at 17:53 +0100, Mark Wielaard wrote:
> > On Wed, 2005-11-09 at 21:16 +0000, Roman Kennke wrote:
> > > Ingo has abstracted out the native code from gnu.java.net.PlainSocketImpl
> > > and gnu.java.net.PlainDatagramSocketImpl into a new VM interface. This
> > > allows VM implementors to provide a different implementation for the
> > > native parts of these classes if they wish. Is this ok to commit as it
> > > is? Do you have any suggestions/improvements to the interface? We would
> > > like to have a stable VM interface for this area, so maybe it would be
> > > helpful to discuss this with other VM implementors...
> > 
> > A first comment is that for the VM reference implementation I would just
> > make all methods native directly, no need to chain them to some helper
> > method here. Is there a reason that only nativeLeave() is synchronized?

I changed that.

> > After we agree on this VM interface (and I think it is good, I just want
> > to try it out first) we can discuss the rest of the patch. Just one
> > quick nitpick now already: please keep the boilerplate at the top of the
> > file intact, some of the files in the patch have the old FSF address.

Fixed.

> - Do we need the boolean stream argument in
>   VMPlainSocketImpl.create()?

No we don't. I have changed the VM interface.

> - VMPlainSocketImpl.bind() has as comment
>   **** How bind to INADDR_ANY ****
>   Should we make a convention for that one? For example just use null
>   add addr?

The API docs for Socket.bind() seem to suggest that null means
INADDR_ANY. So I would do it like this too.

> - There is a PlainSocketImpl.connect() method that takes a timeout
>   argument. We provide a default implementation now, but shouldn't this
>   version move to the VMPlainSocketImpl? I don't know if our default
>   implementation is actually that create, maybe you want to do the
>   actual timeout in a more specific way.

I pulled that into the VM interface.

> - Should we provide wrappers/unwrappers for the Integer and Boolean
>   arguments/return values of setOption() and getOption() that call the
>   VM versions with primitive types? That would make the JNI code much
>   cleaner.

I haven't done this yet because this would mean to change the native
code significantly.

> - PlainSocketImpl.sendUrgentData() should delegate to VMPlainSocketImpl.

Pulled into the VM interface.

> - It might makes sense to have a no argument read() and write() method
>   in the VM interface instead of just the byte[] versions.

Pulled in, however still wraps things up into byte[] to avoid heavy
adjustments in native code at this point.

> - We should explicitly document the new constant IP_TTL we use in
>   PlainDatagramSocketImpl for the VM interface since it isn't defined as
>   SocketOption. Or should we just make a special VM interface to set/get
>   the TTL?

I have pulled the constant into the VM interface.

> - The PlainDatagramSocket multicast opertations join(Group),
>   leave(Group) etc, need to go through to the VM interface.

Pulled in.

Is this ok to check in?

2005-12-16  Roman Kennke  <address@hidden>

        * vm/reference/gnu/java/net/VMPlainSocketImpl.java: New VM
class.
        * vm/reference/gnu/java/net/VMPlainDatagramSocketImpl.java:
        New VM class.
        * native/jni/java-net/gnu_java_net_VMPlainSocketImpl.c: New
file.
        * native/jni/java-net/gnu_java_net_VMPlainDatagramSocketImpl.c:
        New file.
        * native/jni/java-net/gnu_java_net_PlainDatagramSocketImpl.c:
        Removed.
        * native/jni/java-net/gnu_java_net_PlainSocketImpl.c: Removed.
        * native/jni/java-net/Makefile.am: Adjusted for new source
files.
        * gnu/java/net/PlainDatagramSocketImpl.java: Use new VM
interface.
        * gnu/java/net/PlainSocketImpl.java: Use new VM interface.
        * include/gnu_java_net_PlainDatagramSocketImpl.h: Removed.
        * include/gnu_java_net_PlainSocketImpl.h: Removed.
        * include/gnu_java_net_VMPlainDatagramSocketImpl.h: New header
        file.
        * include/gnu_java_net_VMPlainSocketImpl.h: New header file.


/Roman

patch-VM-net.diff.gz
Description: GNU Zip compressed data

signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil