Re: svn commit: r1165084 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/conversion/DateTimeConverters.java base/src/org/ofbiz/base/util/UtilDateTime.java webapp/src/org/ofbiz/webapp/taglib/FormatTag.java

[Adrian Crum-3]

Thank you.

YES, creating a DateFormat instance is expensive. MAYBE this is a
performance issue.

If it is, then you would have to come up with a way to cache a
DateFormat instance per thread (since it isn't thread-safe). Apache
Commons has a version that is supposed to be faster
(http://commons.apache.org/lang/api-2.5/org/apache/commons/lang/time/FastDateFormat.html)
that might be worth considering.

-Adrian

On 9/4/2011 8:07 PM, [hidden email] wrote:

> Author: jleroux
> Date: Sun Sep  4 19:07:16 2011
> New Revision: 1165084
>
> URL: http://svn.apache.org/viewvc?rev=1165084&view=rev
> Log:
> Revert r1165076 (at Adrian's right demand)
>
> Actually I hastily did it bad using static instead of caching. Anyway, I checked there are no real needs to change current code in OFBiz for perf.: one instance is in an exception and the other not called OOTB...
>
> Was:  Closes https://issues.apache.org/jira/browse/OFBIZ-4201
>
> "DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance"
>
> Modified:
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=1165084&r1=1165083&r2=1165084&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java Sun Sep  4 19:07:16 2011
> @@ -554,8 +554,9 @@ public class DateTimeConverters implemen
>                   return new java.sql.Timestamp(df.parse(str).getTime());
>               } catch (ParseException e) {
>                   // before throwing an exception, try a generic format first
> +                df = DateFormat.getDateTimeInstance();
>                   if (timeZone != null) {
> -                    UtilDateTime.DEFAULT_DATE_TIME_FORMAT.setTimeZone(timeZone);
> +                    df.setTimeZone(timeZone);
>                   }
>                   try {
>                       return new java.sql.Timestamp(df.parse(str).getTime());
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java?rev=1165084&r1=1165083&r2=1165084&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java Sun Sep  4 19:07:16 2011
> @@ -69,11 +69,7 @@ public class UtilDateTime {
>       /**
>        * JDBC escape format for java.sql.Time conversions.
>        */
> -
> -    public static final String TIME_FORMAT = "HH:mm:ss";
> -    public static final DateFormat DEFAULT_DATE_TIME_FORMAT = DateFormat.getDateTimeInstance();
> -    public static final DateFormat DEFAULT_DATE_FORMAT = DateFormat.getDateInstance();
> -
> +    public static final String TIME_FORMAT = "HH:mm:ss";
>
>       public static double getInterval(Date from, Date thru) {
>           return thru != null ? thru.getTime() - from.getTime() : 0;
> @@ -703,7 +699,8 @@ public class UtilDateTime {
>       }
>
>       public static String toGmtTimestampString(Timestamp timestamp) {
> -        DEFAULT_DATE_TIME_FORMAT.setTimeZone(TimeZone.getTimeZone("GMT"));
> +        DateFormat df = DateFormat.getDateTimeInstance();
> +        df.setTimeZone(TimeZone.getTimeZone("GMT"));
>           return df.format(timestamp);
>       }
>
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java?rev=1165084&r1=1165083&r2=1165084&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java Sun Sep  4 19:07:16 2011
> @@ -71,7 +71,7 @@ public class FormatTag extends BodyTagSu
>           if (type.charAt(0) == 'N' || type.charAt(0) == 'n')
>               nf = NumberFormat.getNumberInstance();
>           if (type.charAt(0) == 'D' || type.charAt(0) == 'd')
> -            df = org.ofbiz.base.util.UtilDateTime.DEFAULT_DATE_FORMAT;
> +            df = DateFormat.getDateInstance();
>
>           try {
>               if (nf != null) {
>
>

[Jacques Le Roux]

Thanks,

I was cleaning old (what I thought were) simple pending Jiras. Actually I did it too fast in this case (though I vaguely felt the
issue), that why I put the comment about static vs cached.

I think we can use it even with the restriction on TimeZone (we use TimeZone in DateTimeConverters.StringToTimestamp()). But then we
should warn devs about any use of the restricted format (ie  'ZZ' pattern )., an exception could do it.  What do you think?

Jacques

From: "Adrian Crum" <[hidden email]>

> Thank you.
>
> YES, creating a DateFormat instance is expensive. MAYBE this is a performance issue.
>
> If it is, then you would have to come up with a way to cache a DateFormat instance per thread (since it isn't thread-safe). Apache
> Commons has a version that is supposed to be faster
> (http://commons.apache.org/lang/api-2.5/org/apache/commons/lang/time/FastDateFormat.html) that might be worth considering.
>
> -Adrian
>
> On 9/4/2011 8:07 PM, [hidden email] wrote:
>> Author: jleroux
>> Date: Sun Sep  4 19:07:16 2011
>> New Revision: 1165084
>>
>> URL: http://svn.apache.org/viewvc?rev=1165084&view=rev
>> Log:
>> Revert r1165076 (at Adrian's right demand)
>>
>> Actually I hastily did it bad using static instead of caching. Anyway, I checked there are no real needs to change current code
>> in OFBiz for perf.: one instance is in an exception and the other not called OOTB...
>>
>> Was:  Closes https://issues.apache.org/jira/browse/OFBIZ-4201
>>
>> "DateFormat.getDateTimeInstance() is very expensive, we can cache it to improve performance"
>>
>> Modified:
>>      ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>      ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java
>>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=1165084&r1=1165083&r2=1165084&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java Sun Sep  4 19:07:16 2011
>> @@ -554,8 +554,9 @@ public class DateTimeConverters implemen
>>                   return new java.sql.Timestamp(df.parse(str).getTime());
>>               } catch (ParseException e) {
>>                   // before throwing an exception, try a generic format first
>> +                df = DateFormat.getDateTimeInstance();
>>                   if (timeZone != null) {
>> -                    UtilDateTime.DEFAULT_DATE_TIME_FORMAT.setTimeZone(timeZone);
>> +                    df.setTimeZone(timeZone);
>>                   }
>>                   try {
>>                       return new java.sql.Timestamp(df.parse(str).getTime());
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java?rev=1165084&r1=1165083&r2=1165084&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilDateTime.java Sun Sep  4 19:07:16 2011
>> @@ -69,11 +69,7 @@ public class UtilDateTime {
>>       /**
>>        * JDBC escape format for java.sql.Time conversions.
>>        */
>> -
>> -    public static final String TIME_FORMAT = "HH:mm:ss";
>> -    public static final DateFormat DEFAULT_DATE_TIME_FORMAT = DateFormat.getDateTimeInstance();
>> -    public static final DateFormat DEFAULT_DATE_FORMAT = DateFormat.getDateInstance();
>> -
>> +    public static final String TIME_FORMAT = "HH:mm:ss";
>>
>>       public static double getInterval(Date from, Date thru) {
>>           return thru != null ? thru.getTime() - from.getTime() : 0;
>> @@ -703,7 +699,8 @@ public class UtilDateTime {
>>       }
>>
>>       public static String toGmtTimestampString(Timestamp timestamp) {
>> -        DEFAULT_DATE_TIME_FORMAT.setTimeZone(TimeZone.getTimeZone("GMT"));
>> +        DateFormat df = DateFormat.getDateTimeInstance();
>> +        df.setTimeZone(TimeZone.getTimeZone("GMT"));
>>           return df.format(timestamp);
>>       }
>>
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java?rev=1165084&r1=1165083&r2=1165084&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/taglib/FormatTag.java Sun Sep  4 19:07:16 2011
>> @@ -71,7 +71,7 @@ public class FormatTag extends BodyTagSu
>>           if (type.charAt(0) == 'N' || type.charAt(0) == 'n')
>>               nf = NumberFormat.getNumberInstance();
>>           if (type.charAt(0) == 'D' || type.charAt(0) == 'd')
>> -            df = org.ofbiz.base.util.UtilDateTime.DEFAULT_DATE_FORMAT;
>> +            df = DateFormat.getDateInstance();
>>
>>           try {
>>               if (nf != null) {
>>
>>