You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by "Xiaoming Shi (JIRA)" <ji...@apache.org> on 2011/02/25 03:26:41 UTC

[jira] Created: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
------------------------------------------------------------------------------------------

                 Key: PIVOT-708
                 URL: https://issues.apache.org/jira/browse/PIVOT-708
             Project: Pivot
          Issue Type: Bug
          Components: wtk-terra
    Affects Versions: 2.0
            Reporter: Xiaoming Shi


In the file 

./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
line:281

DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.

It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Greg Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003684#comment-13003684 ] 

Greg Brown commented on PIVOT-708:
----------------------------------

Did you do any tests to determine exactly how much of a performance issue calling getDateTimeInstance() is? What about tests to verify that making this change actually improves performance? Note that TerraFileBrowserSkin should never be called from multiple threads anyway, so the multi-threading issues are not relevant.




> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003679#comment-13003679 ] 

Sandro Martini commented on PIVOT-708:
--------------------------------------

some quick tests (and FindBugs) say to me that all seems good, if there aren't objections i can commit, ok ?

Bye


> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Greg Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003692#comment-13003692 ] 

Greg Brown commented on PIVOT-708:
----------------------------------

Sure. The patch seems simple enough.


> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Updated: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sandro Martini updated PIVOT-708:
---------------------------------

    Fix Version/s: 2.0.1

> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Bug
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>             Fix For: 2.0.1
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Updated: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sandro Martini updated PIVOT-708:
---------------------------------

    Attachment: patch.patch

patch, attached here for review

> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Updated: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sandro Martini updated PIVOT-708:
---------------------------------

      Priority: Minor  (was: Major)
      Assignee: Sandro Martini
    Issue Type: Improvement  (was: Bug)

DateFormat and related classes are not Thread safe, so if it's used in a single thread the fix is trivial and we can cache it, otherwise I have to synchronize access to it, but we have to see if it makes sense here.
I'll try the simpler fix as soon as possible, even with a multi-thread test case (if it's the case).

> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003689#comment-13003689 ] 

Sandro Martini commented on PIVOT-708:
--------------------------------------

No, I'm sorry but hadn't time to make some performance tests, but I can say that one occurrence of this is in the method toString(Object row, String columnName) , while the other is in render( ... ) and here I expect some performance improvements.
And I agree with you that there aren't multi-thread issues here, so what do you think for me to commit this change now, and wait next days for me or others (if someone finds time ;-) , otherwise me ) to put some timings inside the class, just to see some number ?

Let me know ...


> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Resolved: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sandro Martini resolved PIVOT-708.
----------------------------------

    Resolution: Fixed

> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (PIVOT-708) DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance

Posted by "Sandro Martini (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/PIVOT-708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003701#comment-13003701 ] 

Sandro Martini commented on PIVOT-708:
--------------------------------------

Ok, so now I resolve this.


> DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance
> ------------------------------------------------------------------------------------------
>
>                 Key: PIVOT-708
>                 URL: https://issues.apache.org/jira/browse/PIVOT-708
>             Project: Pivot
>          Issue Type: Improvement
>          Components: wtk-terra
>    Affects Versions: 2.0
>            Reporter: Xiaoming Shi
>            Assignee: Sandro Martini
>            Priority: Minor
>             Fix For: 2.0.1
>
>         Attachments: patch.patch
>
>
> In the file 
> ./apache-pivot-2.0-src/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraFileBrowserSkin.java 
> line:281
> DateFormat.getDateTimeInstance() is created in the function. We can cache the result by adding a static class field.
> It is similar to the Apache Bug https://issues.apache.org/bugzilla/show_bug.cgi?id=48778

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira