Harmony
  1. Harmony
  2. HARMONY-6116 [classlib] improve performance of java.io.File
  3. HARMONY-6341

[classlib][luni] - implement File.isAbsolute/getAbsolutePath() in java code

    Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 5.0M11
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None

      Description

      File.isAboslute/getAboslutePath() never invoke system call, so it's not necessary to do it via expensive JNI call.

        Activity

        Hide
        Regis Xu added a comment -

        patch serial for this issue.

        Show
        Regis Xu added a comment - patch serial for this issue.
        Hide
        Tim Ellison added a comment -

        Patch 001 for the isAbsolute() change was applied at repo revision r818792.

        Show
        Tim Ellison added a comment - Patch 001 for the isAbsolute() change was applied at repo revision r818792.
        Hide
        Tim Ellison added a comment -

        Patch 003 for removing the getPlatformPath code was applied at repo revision r818803.

        Show
        Tim Ellison added a comment - Patch 003 for removing the getPlatformPath code was applied at repo revision r818803.
        Hide
        Tim Ellison added a comment -

        Regis, can you help me understand the value of patch 002?

        The code seems to be (almost) a copy of the existing code in properPath(boolean), however, it does not set the local variable to cache the absolute path so arguably it is not as helpful?

        Show
        Tim Ellison added a comment - Regis, can you help me understand the value of patch 002? The code seems to be (almost) a copy of the existing code in properPath(boolean), however, it does not set the local variable to cache the absolute path so arguably it is not as helpful?
        Hide
        Regis Xu added a comment -

        Tim, thanks for helping review the patches.

        patch 002 aims to avoid decoding bytes to UTF8 chars (the title should be fixed). Because in the most of cases, absolute path of file is simply $user.dir + path, while old implementation need to get "properPath" which is byte array and then convert to String, even "properPath" has been cached, the converting step is still slower than string concat.

        The code original from "properPath(boolean)" and fix some minor issues to pass the new added test cases. And we can change "properPath(boolean)" to call getAbsolutePath to avoid the duplicate code (I can prepare patch for this).

        Show
        Regis Xu added a comment - Tim, thanks for helping review the patches. patch 002 aims to avoid decoding bytes to UTF8 chars (the title should be fixed). Because in the most of cases, absolute path of file is simply $user.dir + path, while old implementation need to get "properPath" which is byte array and then convert to String, even "properPath" has been cached, the converting step is still slower than string concat. The code original from "properPath(boolean)" and fix some minor issues to pass the new added test cases. And we can change "properPath(boolean)" to call getAbsolutePath to avoid the duplicate code (I can prepare patch for this).
        Hide
        Tim Ellison added a comment -

        I looked again, and had a play with writing more test cases to see if I could break things

        All these tests pass for me on the RI on Windows, but not all pass on Harmony. I haven't finished with Unix yet.
        see "getAbsolute-tim-1.patch"

        Show
        Tim Ellison added a comment - I looked again, and had a play with writing more test cases to see if I could break things All these tests pass for me on the RI on Windows, but not all pass on Harmony. I haven't finished with Unix yet. see "getAbsolute-tim-1.patch"
        Hide
        Mark Hindess added a comment -

        Tim, did you make any more progress with this?

        Show
        Mark Hindess added a comment - Tim, did you make any more progress with this?

          People

          • Assignee:
            Tim Ellison
            Reporter:
            Regis Xu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development