Re: PATCH for java.nio FileChannel and MappedByteBuffer

[Michael Koch <konqueror at gmx dot de>]

Hi Per, hi list,

Am Freitag, 20. Februar 2004 02:43 schrieb Per Bothner:
> This is the last part of my java.nio patches.
> The executive summary:
>
> Working implementations of mapped I/o (at least
> read mode) on Posix and Win32.  [At least it
> worked for earlier versions of the patch.]
>
> FileDescriptor is now implemented in terms of
> FileChannelImpl rather than the other way round.
> Thus FileInputStream etc are implemented using
> FileChannelImpl rather than FileDescriptor.
> Hence FileChannelImpl had to be moved to the
> gnu.java hierarchy, so it could be public.
>
> The main reason for this change is that java.nio
> is intended for more advanced and/or higher-performance
> I/O needs, so it it seems wasteful to have them run
> on top of the older FileDescriptor code.
>
> You may also note that I don't use symlinks to
> select between the natFileChannel{Posix,Win32,Ecos}.cc,
> but instead use teh pre-processor.

Some small comments: 

- You include ChangeLog in diff which should be avoided
- You include generated files in the diff which should be avoided

These both are really minor and are more hints then anything else.

- You move some files around. That makes it more difficult to detect 
changes in them. Can you do the move in one patch and the changes to 
them in another ?
- I dont like the inclusion of the natFileChannel{Posix,Win32,Ecos}.cc. 
We use the old way for a long time and had no real reason to change 
this. We spoke on IRC about this and nobody seemed to like it too.
Can you please revert this change ?
- I thought about your changes and I still dont think that changing 
FileDescriptor to utilize FileChannel. As I said before the java.net 
socket impl classes use a FileDescriptor to store its socket handle.
It seems logical wrong to put a FileChannel object into FileDescriptor.

Can you somehow break this into some smaller patches ?


Michael