You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2011/09/04 21:07:16 UTC
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
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) {
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
Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ad...@sandglass-software.com>
> 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, jleroux@apache.org 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) {
>>
>>
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
Posted by Adrian Crum <ad...@sandglass-software.com>.
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, jleroux@apache.org 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) {
>
>