Casey Marshall wrote:
> What I'd like people to look at and comment on specifically are the
> changes to gnu.java.nio.VMChannel and gnu.java.net.VMPlainSocketImpl,
> for opinions on how well these classes (the public methods) help
> abstract away platform-specific code.
Thanks. I like the idea of the patch, but I have quite a few comments
about the current implementation. As a reminder: Please don't take my
directness the wrong way, English is not my native language and I'm
rather blunt, even for a Dutchman.
Here are most of them:
- The NativeFD interface should have a better name (e.g. something like
VMChannelOwner).
- I don't like the fact that you're introducing allocation in virtually
every I/O path.
- Why "vmch.getNativeFD().close();" instead of "vmch.close();"?
- Most classes should not have a finalize method, only VMChannel should
have a finalize method.
- All valid socket options should be delegated to impl (and if any
aren't yet implemented, the impl should throw the
UnsupportedOperationException).
- Method naming could be better ("return
ch.getVMChannel().getNativeFD().getNativeFD();").
- Instead of changing FileInputStream/FileOutputStream/RandomAccessFile,
FileChannelImpl should accommodate them.
- PlainSocketImpl uses ByteBuffer.allocateDirect to allocate the two
single byte buffers (oneByteRead and oneByteWrite, which aren't used
BTW), I'd like to see the decision to use allocate vs. allocateDirect
delegated to VMPlainSocketImpl. (For me allocateDirect is actually less
efficient in this case).
- PlainSocketImpl.connect should simply call
impl.connect(address,timeout) which in turn calls
channel.connect(address,timeout), the decision how to implement the
timeout should be in the VM layer.
- PlainSocketImpl.write(byte[],int,int) implementation is unacceptable.
Thanks,
Jeroen
