Harmony
  1. Harmony
  2. HARMONY-42

com.ibm.io.nio.FileChannel is not fully implemented

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None

      Description

      Many functions of FileChannel, such as memory map, transfer, gathering/scattering I/O are not implemented. Further, three classes in java.io, FileInputStream FileOutputStream, and RandomAccessFile, are related to java.nio.FileChannel, so that they can be refactored to base on same JNI interface, just like the network channels and sockets.

      1. IFileSystem.java
        3 kB
        Paulex Yang
      2. Harmony42-java_io_FileOutputStream-patch.txt
        6 kB
        Paulex Yang
      3. Harmony42-java_io_FileChannelFactory-patch.txt
        2 kB
        Paulex Yang
      4. Harmony42-IMemorySystem-patch.txt
        1 kB
        Paulex Yang
      5. Harmony42-IFileSystem-patch.txt
        3 kB
        Paulex Yang
      6. FileChannel_patch.zip
        45 kB
        Richard Liang

        Activity

        Hide
        Paulex Yang added a comment -

        Here goes my proposal of the JNI interfaces, which can be the base of both FileChannel and the File I/O classes in java.io.

        Show
        Paulex Yang added a comment - Here goes my proposal of the JNI interfaces, which can be the base of both FileChannel and the File I/O classes in java.io.
        Hide
        Tim Ellison added a comment -

        Paulex,

        I don't understand this issue. The file you have attached is a modified version of the com.ibm.platform.IFileSystem.java that is in NIO. Your comment indicates that it is enhanced to support regular IO types, yet it appears that mmap is removed. Is that right?

        I'm not going to take this enhancement at the moment. Please provide further rationale.

        (Please can you send your enhancements as diff's rather than file replacements, it will make tehm much easier to evaluate.)

        Thanks,
        Tim

        Show
        Tim Ellison added a comment - Paulex, I don't understand this issue. The file you have attached is a modified version of the com.ibm.platform.IFileSystem.java that is in NIO. Your comment indicates that it is enhanced to support regular IO types, yet it appears that mmap is removed. Is that right? I'm not going to take this enhancement at the moment. Please provide further rationale. (Please can you send your enhancements as diff's rather than file replacements, it will make tehm much easier to evaluate.) Thanks, Tim
        Hide
        Paulex Yang added a comment -

        Sorry that I didn't tell the whole story, the mmap related functions are moved to IMemorySystem, because most of other mmap related functions, such as load, isLoad and flush are all memory operations, and I think the IMemorySystem is more appropriate place to hold them.

        About the IFileSystem itself, 4 methods are modifed, all of them are not fully implemented in current Harmony code base, and 3 methods are added. Diff patches are attached for your detail information.

        Show
        Paulex Yang added a comment - Sorry that I didn't tell the whole story, the mmap related functions are moved to IMemorySystem, because most of other mmap related functions, such as load, isLoad and flush are all memory operations, and I think the IMemorySystem is more appropriate place to hold them. About the IFileSystem itself, 4 methods are modifed, all of them are not fully implemented in current Harmony code base, and 3 methods are added. Diff patches are attached for your detail information.
        Hide
        Paulex Yang added a comment -

        A POC refactor patch for java.io.FileOutputStream is attached, with some minor changes on FileChannelFactory. java.io.FileInputStream and RandomAccessFile can be easily refactored in similiar style.

        As a completement to my comment on the issue 40, The FileChannelImpl's implCloseChannel() can look like:
        protected void implCloseChannel() throws IOException {
        if(stream instanceof Closeable)

        { ((Closeable)stream).close(); }

        fileSystem.close(handle);
        }

        and the FileChannelImpl's super class, AbstractInterruptibleChannel's close method can be:

        public synchronized final void close() throws IOException {
        if (!closed)

        { closed = true; implCloseChannel(); }


        }

        Show
        Paulex Yang added a comment - A POC refactor patch for java.io.FileOutputStream is attached, with some minor changes on FileChannelFactory. java.io.FileInputStream and RandomAccessFile can be easily refactored in similiar style. As a completement to my comment on the issue 40, The FileChannelImpl's implCloseChannel() can look like: protected void implCloseChannel() throws IOException { if(stream instanceof Closeable) { ((Closeable)stream).close(); } fileSystem.close(handle); } and the FileChannelImpl's super class, AbstractInterruptibleChannel's close method can be: public synchronized final void close() throws IOException { if (!closed) { closed = true; implCloseChannel(); } }
        Hide
        Richard Liang added a comment -

        Hello Tim,

        Finally, The refactor of FileChannel and related classes are completed , as well as some modification about
        some native code (OS File system and OS Memory system). Would you please help to verify my patches? Thanks a lot.

        Hopefully, I will post my test cases soon

        Show
        Richard Liang added a comment - Hello Tim, Finally, The refactor of FileChannel and related classes are completed , as well as some modification about some native code (OS File system and OS Memory system). Would you please help to verify my patches? Thanks a lot. Hopefully, I will post my test cases soon
        Hide
        Tim Ellison added a comment -

        Richard, just to let you know that I'm currently looking at this refactoring – it will take me a while to get through it so please be patient<g>.

        Show
        Tim Ellison added a comment - Richard, just to let you know that I'm currently looking at this refactoring – it will take me a while to get through it so please be patient<g>.
        Hide
        Tim Ellison added a comment -

        Paulex / Richard,

        I've had a look at the patches you sent and applied them at repo revision 376690.

        I have the following comments (note especially #6):

        1. Added a missing ASF copyright statment to IMemorySystem.h,
        2. nio/OSFileSystemWin32.c#OSFileSystem_transferImpl has a number of printf's in there that should be removed (e.g. printf("Error getJavaIoFileDescriptorContentsAsAPointer!\n"); )
        3. same thing in port/hymmap.c#hymmap_map_filehandler (e.g. printf("print this to show in this function"); ) – this also has a very strange code layout !
        4. I have added some missing exports from the exp & def files for the new JNI functions you defined
        5. I see a few TODO's in there, like IPv6 support on a couple fo functions – that's ok, but if you do the work please send it along.
        6. I have not applied the modifications you made to the portlib to extend the mmap functionality; and therefore I have modified the OSMemory#mmap method to throw a NotYetImplementedException. I'll discuss this on the mailing list.

        Please open a new JIRA for further patches/work in this area – and thank you very much for your contribution.

        Show
        Tim Ellison added a comment - Paulex / Richard, I've had a look at the patches you sent and applied them at repo revision 376690. I have the following comments (note especially #6): 1. Added a missing ASF copyright statment to IMemorySystem.h, 2. nio/OSFileSystemWin32.c#OSFileSystem_transferImpl has a number of printf's in there that should be removed (e.g. printf("Error getJavaIoFileDescriptorContentsAsAPointer!\n"); ) 3. same thing in port/hymmap.c#hymmap_map_filehandler (e.g. printf("print this to show in this function"); ) – this also has a very strange code layout ! 4. I have added some missing exports from the exp & def files for the new JNI functions you defined 5. I see a few TODO's in there, like IPv6 support on a couple fo functions – that's ok, but if you do the work please send it along. 6. I have not applied the modifications you made to the portlib to extend the mmap functionality; and therefore I have modified the OSMemory#mmap method to throw a NotYetImplementedException. I'll discuss this on the mailing list. Please open a new JIRA for further patches/work in this area – and thank you very much for your contribution.
        Hide
        Richard Liang added a comment -

        Thanks a lot, Tim. We are satisfied with the patch application. You may close this JIRA.

        Show
        Richard Liang added a comment - Thanks a lot, Tim. We are satisfied with the patch application. You may close this JIRA.
        Hide
        Tim Ellison added a comment -

        Verified by Richard.

        Show
        Tim Ellison added a comment - Verified by Richard.

          People

          • Assignee:
            Tim Ellison
            Reporter:
            Paulex Yang
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development