You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by qi...@apache.org on 2008/08/26 10:59:55 UTC
svn commit: r689001 - in
/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql:
Date.java Time.java Timestamp.java
Author: qiuxx
Date: Tue Aug 26 01:59:50 2008
New Revision: 689001
URL: http://svn.apache.org/viewvc?rev=689001&view=rev
Log:
Apply for HARMONY-5958,([classlib][sql][performance] - improve performance of java.sql.Date/Time/Timestamp.toString())
Modified:
harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java (original)
+++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java Tue Aug 26 01:59:50 2008
@@ -17,8 +17,6 @@
package java.sql;
-import java.text.SimpleDateFormat;
-
/**
* A Date class which can consume and produce dates in SQL Date format.
* <p>
@@ -175,8 +173,28 @@
*/
@Override
public String toString() {
- SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
- return dateFormat.format(this);
+ StringBuilder sb = new StringBuilder(10);
+
+ format((getYear() + 1900), 4, sb);
+ sb.append('-');
+ format((getMonth() + 1), 2, sb);
+ sb.append('-');
+ format(getDate(), 2, sb);
+
+ return sb.toString();
+ }
+
+ private static final String PADDING = "0000"; //$NON-NLS-1$
+
+ /*
+ * Private method to format the time
+ */
+ private void format(int date, int digits, StringBuilder sb) {
+ String str = String.valueOf(date);
+ if (digits - str.length() > 0) {
+ sb.append(PADDING.substring(0, digits - str.length()));
+ }
+ sb.append(str);
}
/**
Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java (original)
+++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java Tue Aug 26 01:59:50 2008
@@ -17,7 +17,6 @@
package java.sql;
-import java.text.SimpleDateFormat;
import java.util.Date;
/**
@@ -180,8 +179,28 @@
*/
@Override
public String toString() {
- SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss"); //$NON-NLS-1$
- return dateFormat.format(this);
+ StringBuilder sb = new StringBuilder(8);
+
+ format(getHours(), 2, sb);
+ sb.append(':');
+ format(getMinutes(), 2, sb);
+ sb.append(':');
+ format(getSeconds(), 2, sb);
+
+ return sb.toString();
+ }
+
+ private static final String PADDING = "00"; //$NON-NLS-1$
+
+ /*
+ * Private method to format the time
+ */
+ private void format(int date, int digits, StringBuilder sb) {
+ String str = String.valueOf(date);
+ if (digits - str.length() > 0) {
+ sb.append(PADDING.substring(0, digits - str.length()));
+ }
+ sb.append(str);
}
/**
Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java (original)
+++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java Tue Aug 26 01:59:50 2008
@@ -309,62 +309,43 @@
@SuppressWarnings("deprecation")
@Override
public String toString() {
- /*
- * Use a DecimalFormat to lay out the nanosecond value as a simple
- * string of 9 integers, with leading Zeros
- */
- DecimalFormat decimalFormat = new DecimalFormat("0"); //$NON-NLS-1$
- decimalFormat.setMinimumIntegerDigits(9);
- decimalFormat.setMaximumIntegerDigits(9);
- String theNanos = decimalFormat.format(nanos);
- theNanos = stripTrailingZeros(theNanos);
-
- String year = format((getYear() + 1900), 4);
- String month = format((getMonth() + 1), 2);
- String date = format(getDate(), 2);
- String hours = format(getHours(), 2);
- String minutes = format(getMinutes(), 2);
- String seconds = format(getSeconds(), 2);
-
- return year + '-' + month + '-' + date + ' ' + hours + ':' + minutes
- + ':' + seconds + '.' + theNanos;
- }
+ StringBuilder sb = new StringBuilder(29);
- /*
- * Private method to format the time
- */
- private String format(int date, int digits) {
- StringBuilder dateStringBuffer = new StringBuilder(String.valueOf(date));
- while (dateStringBuffer.length() < digits) {
- dateStringBuffer = dateStringBuffer.insert(0, '0');
+ format((getYear() + 1900), 4, sb);
+ sb.append('-');
+ format((getMonth() + 1), 2, sb);
+ sb.append('-');
+ format(getDate(), 2, sb);
+ sb.append(' ');
+ format(getHours(), 2, sb);
+ sb.append(':');
+ format(getMinutes(), 2, sb);
+ sb.append(':');
+ format(getSeconds(), 2, sb);
+ sb.append('.');
+ if (nanos == 0) {
+ sb.append('0');
+ } else {
+ format(nanos, 9, sb);
+ while (sb.charAt(sb.length() - 1) == '0') {
+ sb.setLength(sb.length() - 1);
+ }
}
- return dateStringBuffer.toString();
+
+ return sb.toString();
}
- /*
- * Private method to strip trailing '0' characters from a string. @param
- * inputString the starting string @return a string with the trailing zeros
- * stripped - will leave a single 0 at the beginning of the string
- */
- private String stripTrailingZeros(String inputString) {
- String finalString;
+ private static final String PADDING = "000000000"; //$NON-NLS-1$
- int i;
- for (i = inputString.length(); i > 0; i--) {
- if (inputString.charAt(i - 1) != '0') {
- break;
- }
- /*
- * If the string has a 0 as its first character, return a string
- * with a single '0'
- */
- if (i == 1) {
- return "0"; //$NON-NLS-1$
- }
+ /*
+ * Private method to format the time
+ */
+ private void format(int date, int digits, StringBuilder sb) {
+ String str = String.valueOf(date);
+ if (digits - str.length() > 0) {
+ sb.append(PADDING.substring(0, digits - str.length()));
}
-
- finalString = inputString.substring(0, i);
- return finalString;
+ sb.append(str);
}
/**
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nd...@apache.org>.
Hmm...(just performed some tests)...indeed it is. That boggles the
mind. That's certinaly not intuitive. I'm going to have to remember
that one.
The tests are fine then.
-Nathan
On Mon, Sep 1, 2008 at 7:57 PM, Tony Wu <wu...@gmail.com> wrote:
> Hi, Nathan
> The format pattern is timezone independent but the value might be
> different. For instance, the time 2008-09-01 08:00:00 in GMT should be
> 2008-09-01 16:00:00 in CTT(GMT+8).
>
>
> On 9/2/08, Nathan Beyer <nd...@apache.org> wrote:
>> Since the classes are supposed to be TimeZone-independant, couldn't
>> all of the TimeZone code be removed?
>>
>> -Nathan
>>
>> On Mon, Sep 1, 2008 at 12:19 AM, Tony Wu <wu...@gmail.com> wrote:
>> > I reviewed these 3 tests. It's true that locales other than GMT have
>> > been covered in the TimeTest and TimeStampTest but not in DateTest.
>> > And the comments in the testToString is confusing ...
>> > Also some testcases in TimestampTest have dependency to each other.
>> > Since other locales have been covered by testToString, I just
>> > explicity set the default locale to GMT at the beginning of these
>> > testcases.
>> >
>> > I've corrected them at r690847.
>> >
>> > On Sat, Aug 30, 2008 at 10:22 AM, Nathan Beyer <nd...@apache.org> wrote:
>> >> But, the tests are invalid, correct? At a minimum, all of the TimeZone
>> >> code in the tests is unnecessary.
>> >>
>> >> I'm not trying to insinuate or infer that the patch is wrong. I just
>> >> like to look at each code change in a larger context. In this case,
>> >> the tests are still passing, but upon closer inspection of the tests,
>> >> they seem to be invalid or, as mentioned above, they have unnecessary
>> >> comments and seemingly invalid comments.
>> >>
>> >> -Nathan
>> >>
>> >> On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wu...@gmail.com> wrote:
>> >>> I think so. The format has been explicitly specified by spec, such as
>> >>> yyyy-mm-dd hh:mm:ss.fffffffff
>> >>>
>> >>> Thus it should be locale independent and this patch is valid.
>> >>>
>> >>> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
>> >>>> I looked at the test in further detail and they seem okay. The tests
>> >>>> seem odd though, as they set TimeZone values and mention things like
>> >>>> TimeZone affecting the output. That shouldn't be the case though,
>> >>>> correct? The formats are all in UTC/GMT, correct?
>> >>>>
>> >>>> -Nathan
>> >>>>
>> >>>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
>> >>>>> As Regis said, there exist tests for these toString method, which,
>> >>>>> IMHO, are covering this modfication.
>> >>>>> Shall we add more tests?
>> >>>>>
>> >>>>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>> >>>>>> I was referring to a functional test, not a performance test.
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>> >>>>>>> Here is a small test from Regis:
>> >>>>>>>
>> >>>>>>> ----------
>> >>>>>>> int count = 5000;
>> >>>>>>>
>> >>>>>>> for (int i = 0; i < count; ++i) { new Date(new
>> >>>>>>> java.util.Date().getTime()).toString(); }
>> >>>>>>>
>> >>>>>>> Date date = new Date(new java.util.Date().getTime());
>> >>>>>>> long start = System.currentTimeMillis();
>> >>>>>>>
>> >>>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
>> >>>>>>> long end = System.currentTimeMillis();
>> >>>>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>> >>>>>>> " + (end - start));
>> >>>>>>>
>> >>>>>>> Time time = new Time(new java.util.Date().getTime());
>> >>>>>>> start = System.currentTimeMillis();
>> >>>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
>> >>>>>>> end = System.currentTimeMillis();
>> >>>>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>> >>>>>>> " + (end - start));
>> >>>>>>>
>> >>>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>> >>>>>>> start = System.currentTimeMillis();
>> >>>>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>> >>>>>>> end = System.currentTimeMillis();
>> >>>>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>> >>>>>>> cost: " + (end - start));
>> >>>>>>> -------------
>> >>>>>>>
>> >>>>>>> the first loop, give time for jvm to warm up
>> >>>>>>>
>> >>>>>>> Below data compare the two implementations:
>> >>>>>>>
>> >>>>>>> before the patch:
>> >>>>>>> Invoke Date.toString() 5000 times, cost: 6757
>> >>>>>>> Invoke Time.toString() 5000 times, cost: 7699
>> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>> >>>>>>>
>> >>>>>>> after the patch:
>> >>>>>>> Invoke Date.toString() 5000 times, cost: 84
>> >>>>>>> Invoke Time.toString() 5000 times, cost: 95
>> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> We can gain obvious improvement.
>> >>>>>>>
>> >>>>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>> >>>>>>>> Are there any associated test cases with this change? On a quick
>> >>>>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>> >>>>>>>> not, we need to start requiring some test cases for "simple
>> >>>>>>>> improvements" like this to continue functional correctness.
>> >>>>>>>>
>> >>>>>>>> -Nathan
>> >>>>>>>>
>> >>>>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>> >>>>>>>>> Author: qiuxx
>> >>>>>>>>> Date: Tue Aug 26 01:59:50 2008
>> >>>>>>>>> New Revision: 689001
>> >>>>>>>>>
>> >>>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>> >>>>>>>>> Log:
>> >>>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>> >>>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>> >>>>>>>>>
>> >>>>>>>>> Modified:
>> >>>>>>>>>
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> >>>>>>>>>
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> >>>>>>>>>
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> >>>>>>>>>
>> >>>>>>>>> Modified:
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> >>>>>>>>> URL:
>> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>> >>>>>>>>> ==============================================================================
>> >>>>>>>>> ---
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> >>>>>>>>> (original)
>> >>>>>>>>> +++
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> >>>>>>>>> Tue Aug 26 01:59:50 2008
>> >>>>>>>>> @@ -17,8 +17,6 @@
>> >>>>>>>>>
>> >>>>>>>>> package java.sql;
>> >>>>>>>>>
>> >>>>>>>>> -import java.text.SimpleDateFormat;
>> >>>>>>>>> -
>> >>>>>>>>> /**
>> >>>>>>>>> * A Date class which can consume and produce dates in SQL Date format.
>> >>>>>>>>> * <p>
>> >>>>>>>>> @@ -175,8 +173,28 @@
>> >>>>>>>>> */
>> >>>>>>>>> @Override
>> >>>>>>>>> public String toString() {
>> >>>>>>>>> - SimpleDateFormat dateFormat = new
>> >>>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>> >>>>>>>>> - return dateFormat.format(this);
>> >>>>>>>>> + StringBuilder sb = new StringBuilder(10);
>> >>>>>>>>> +
>> >>>>>>>>> + format((getYear() + 1900), 4, sb);
>> >>>>>>>>> + sb.append('-');
>> >>>>>>>>> + format((getMonth() + 1), 2, sb);
>> >>>>>>>>> + sb.append('-');
>> >>>>>>>>> + format(getDate(), 2, sb);
>> >>>>>>>>> +
>> >>>>>>>>> + return sb.toString();
>> >>>>>>>>> + }
>> >>>>>>>>> +
>> >>>>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>> >>>>>>>>> +
>> >>>>>>>>> + /*
>> >>>>>>>>> + * Private method to format the time
>> >>>>>>>>> + */
>> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>> >>>>>>>>> + String str = String.valueOf(date);
>> >>>>>>>>> + if (digits - str.length() > 0) {
>> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>> >>>>>>>>> + }
>> >>>>>>>>> + sb.append(str);
>> >>>>>>>>> }
>> >>>>>>>>>
>> >>>>>>>>> /**
>> >>>>>>>>>
>> >>>>>>>>> Modified:
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> >>>>>>>>> URL:
>> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>> >>>>>>>>> ==============================================================================
>> >>>>>>>>> ---
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> >>>>>>>>> (original)
>> >>>>>>>>> +++
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> >>>>>>>>> Tue Aug 26 01:59:50 2008
>> >>>>>>>>> @@ -17,7 +17,6 @@
>> >>>>>>>>>
>> >>>>>>>>> package java.sql;
>> >>>>>>>>>
>> >>>>>>>>> -import java.text.SimpleDateFormat;
>> >>>>>>>>> import java.util.Date;
>> >>>>>>>>>
>> >>>>>>>>> /**
>> >>>>>>>>> @@ -180,8 +179,28 @@
>> >>>>>>>>> */
>> >>>>>>>>> @Override
>> >>>>>>>>> public String toString() {
>> >>>>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>> >>>>>>>>> //$NON-NLS-1$
>> >>>>>>>>> - return dateFormat.format(this);
>> >>>>>>>>> + StringBuilder sb = new StringBuilder(8);
>> >>>>>>>>> +
>> >>>>>>>>> + format(getHours(), 2, sb);
>> >>>>>>>>> + sb.append(':');
>> >>>>>>>>> + format(getMinutes(), 2, sb);
>> >>>>>>>>> + sb.append(':');
>> >>>>>>>>> + format(getSeconds(), 2, sb);
>> >>>>>>>>> +
>> >>>>>>>>> + return sb.toString();
>> >>>>>>>>> + }
>> >>>>>>>>> +
>> >>>>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>> >>>>>>>>> +
>> >>>>>>>>> + /*
>> >>>>>>>>> + * Private method to format the time
>> >>>>>>>>> + */
>> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>> >>>>>>>>> + String str = String.valueOf(date);
>> >>>>>>>>> + if (digits - str.length() > 0) {
>> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>> >>>>>>>>> + }
>> >>>>>>>>> + sb.append(str);
>> >>>>>>>>> }
>> >>>>>>>>>
>> >>>>>>>>> /**
>> >>>>>>>>>
>> >>>>>>>>> Modified:
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> >>>>>>>>> URL:
>> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>> >>>>>>>>> ==============================================================================
>> >>>>>>>>> ---
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> >>>>>>>>> (original)
>> >>>>>>>>> +++
>> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> >>>>>>>>> Tue Aug 26 01:59:50 2008
>> >>>>>>>>> @@ -309,62 +309,43 @@
>> >>>>>>>>> @SuppressWarnings("deprecation")
>> >>>>>>>>> @Override
>> >>>>>>>>> public String toString() {
>> >>>>>>>>> - /*
>> >>>>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>> >>>>>>>>> simple
>> >>>>>>>>> - * string of 9 integers, with leading Zeros
>> >>>>>>>>> - */
>> >>>>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>> >>>>>>>>> //$NON-NLS-1$
>> >>>>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>> >>>>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>> >>>>>>>>> - String theNanos = decimalFormat.format(nanos);
>> >>>>>>>>> - theNanos = stripTrailingZeros(theNanos);
>> >>>>>>>>> -
>> >>>>>>>>> - String year = format((getYear() + 1900), 4);
>> >>>>>>>>> - String month = format((getMonth() + 1), 2);
>> >>>>>>>>> - String date = format(getDate(), 2);
>> >>>>>>>>> - String hours = format(getHours(), 2);
>> >>>>>>>>> - String minutes = format(getMinutes(), 2);
>> >>>>>>>>> - String seconds = format(getSeconds(), 2);
>> >>>>>>>>> -
>> >>>>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>> >>>>>>>>> minutes
>> >>>>>>>>> - + ':' + seconds + '.' + theNanos;
>> >>>>>>>>> - }
>> >>>>>>>>> + StringBuilder sb = new StringBuilder(29);
>> >>>>>>>>>
>> >>>>>>>>> - /*
>> >>>>>>>>> - * Private method to format the time
>> >>>>>>>>> - */
>> >>>>>>>>> - private String format(int date, int digits) {
>> >>>>>>>>> - StringBuilder dateStringBuffer = new
>> >>>>>>>>> StringBuilder(String.valueOf(date));
>> >>>>>>>>> - while (dateStringBuffer.length() < digits) {
>> >>>>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>> >>>>>>>>> + format((getYear() + 1900), 4, sb);
>> >>>>>>>>> + sb.append('-');
>> >>>>>>>>> + format((getMonth() + 1), 2, sb);
>> >>>>>>>>> + sb.append('-');
>> >>>>>>>>> + format(getDate(), 2, sb);
>> >>>>>>>>> + sb.append(' ');
>> >>>>>>>>> + format(getHours(), 2, sb);
>> >>>>>>>>> + sb.append(':');
>> >>>>>>>>> + format(getMinutes(), 2, sb);
>> >>>>>>>>> + sb.append(':');
>> >>>>>>>>> + format(getSeconds(), 2, sb);
>> >>>>>>>>> + sb.append('.');
>> >>>>>>>>> + if (nanos == 0) {
>> >>>>>>>>> + sb.append('0');
>> >>>>>>>>> + } else {
>> >>>>>>>>> + format(nanos, 9, sb);
>> >>>>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>> >>>>>>>>> + sb.setLength(sb.length() - 1);
>> >>>>>>>>> + }
>> >>>>>>>>> }
>> >>>>>>>>> - return dateStringBuffer.toString();
>> >>>>>>>>> +
>> >>>>>>>>> + return sb.toString();
>> >>>>>>>>> }
>> >>>>>>>>>
>> >>>>>>>>> - /*
>> >>>>>>>>> - * Private method to strip trailing '0' characters from a string.
>> >>>>>>>>> @param
>> >>>>>>>>> - * inputString the starting string @return a string with the
>> >>>>>>>>> trailing zeros
>> >>>>>>>>> - * stripped - will leave a single 0 at the beginning of the string
>> >>>>>>>>> - */
>> >>>>>>>>> - private String stripTrailingZeros(String inputString) {
>> >>>>>>>>> - String finalString;
>> >>>>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>> >>>>>>>>>
>> >>>>>>>>> - int i;
>> >>>>>>>>> - for (i = inputString.length(); i > 0; i--) {
>> >>>>>>>>> - if (inputString.charAt(i - 1) != '0') {
>> >>>>>>>>> - break;
>> >>>>>>>>> - }
>> >>>>>>>>> - /*
>> >>>>>>>>> - * If the string has a 0 as its first character, return a
>> >>>>>>>>> string
>> >>>>>>>>> - * with a single '0'
>> >>>>>>>>> - */
>> >>>>>>>>> - if (i == 1) {
>> >>>>>>>>> - return "0"; //$NON-NLS-1$
>> >>>>>>>>> - }
>> >>>>>>>>> + /*
>> >>>>>>>>> + * Private method to format the time
>> >>>>>>>>> + */
>> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>> >>>>>>>>> + String str = String.valueOf(date);
>> >>>>>>>>> + if (digits - str.length() > 0) {
>> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>> >>>>>>>>> }
>> >>>>>>>>> -
>> >>>>>>>>> - finalString = inputString.substring(0, i);
>> >>>>>>>>> - return finalString;
>> >>>>>>>>> + sb.append(str);
>> >>>>>>>>> }
>> >>>>>>>>>
>> >>>>>>>>> /**
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> --
>> >>>>>>> Best Regards
>> >>>>>>> Sean, Xiao Xia Qiu
>> >>>>>>>
>> >>>>>>> China Software Development Lab, IBM
>> >>>>>>>
>> >>>>>>
>> >>>>>> --
>> >>>>>> Sent from Gmail for mobile | mobile.google.com
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> Best Regards
>> >>>>> Sean, Xiao Xia Qiu
>> >>>>>
>> >>>>> China Software Development Lab, IBM
>> >>>>>
>> >>>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Tony Wu
>> >>> China Software Development Lab, IBM
>> >>>
>> >>
>> >
>> >
>> >
>> > --
>> > Tony Wu
>> > China Software Development Lab, IBM
>> >
>>
>
>
> --
> Tony Wu
> China Software Development Lab, IBM
>
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Tony Wu <wu...@gmail.com>.
Hi, Nathan
The format pattern is timezone independent but the value might be
different. For instance, the time 2008-09-01 08:00:00 in GMT should be
2008-09-01 16:00:00 in CTT(GMT+8).
On 9/2/08, Nathan Beyer <nd...@apache.org> wrote:
> Since the classes are supposed to be TimeZone-independant, couldn't
> all of the TimeZone code be removed?
>
> -Nathan
>
> On Mon, Sep 1, 2008 at 12:19 AM, Tony Wu <wu...@gmail.com> wrote:
> > I reviewed these 3 tests. It's true that locales other than GMT have
> > been covered in the TimeTest and TimeStampTest but not in DateTest.
> > And the comments in the testToString is confusing ...
> > Also some testcases in TimestampTest have dependency to each other.
> > Since other locales have been covered by testToString, I just
> > explicity set the default locale to GMT at the beginning of these
> > testcases.
> >
> > I've corrected them at r690847.
> >
> > On Sat, Aug 30, 2008 at 10:22 AM, Nathan Beyer <nd...@apache.org> wrote:
> >> But, the tests are invalid, correct? At a minimum, all of the TimeZone
> >> code in the tests is unnecessary.
> >>
> >> I'm not trying to insinuate or infer that the patch is wrong. I just
> >> like to look at each code change in a larger context. In this case,
> >> the tests are still passing, but upon closer inspection of the tests,
> >> they seem to be invalid or, as mentioned above, they have unnecessary
> >> comments and seemingly invalid comments.
> >>
> >> -Nathan
> >>
> >> On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wu...@gmail.com> wrote:
> >>> I think so. The format has been explicitly specified by spec, such as
> >>> yyyy-mm-dd hh:mm:ss.fffffffff
> >>>
> >>> Thus it should be locale independent and this patch is valid.
> >>>
> >>> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
> >>>> I looked at the test in further detail and they seem okay. The tests
> >>>> seem odd though, as they set TimeZone values and mention things like
> >>>> TimeZone affecting the output. That shouldn't be the case though,
> >>>> correct? The formats are all in UTC/GMT, correct?
> >>>>
> >>>> -Nathan
> >>>>
> >>>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
> >>>>> As Regis said, there exist tests for these toString method, which,
> >>>>> IMHO, are covering this modfication.
> >>>>> Shall we add more tests?
> >>>>>
> >>>>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
> >>>>>> I was referring to a functional test, not a performance test.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
> >>>>>>> Here is a small test from Regis:
> >>>>>>>
> >>>>>>> ----------
> >>>>>>> int count = 5000;
> >>>>>>>
> >>>>>>> for (int i = 0; i < count; ++i) { new Date(new
> >>>>>>> java.util.Date().getTime()).toString(); }
> >>>>>>>
> >>>>>>> Date date = new Date(new java.util.Date().getTime());
> >>>>>>> long start = System.currentTimeMillis();
> >>>>>>>
> >>>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
> >>>>>>> long end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
> >>>>>>> " + (end - start));
> >>>>>>>
> >>>>>>> Time time = new Time(new java.util.Date().getTime());
> >>>>>>> start = System.currentTimeMillis();
> >>>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
> >>>>>>> end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
> >>>>>>> " + (end - start));
> >>>>>>>
> >>>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
> >>>>>>> start = System.currentTimeMillis();
> >>>>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
> >>>>>>> end = System.currentTimeMillis();
> >>>>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
> >>>>>>> cost: " + (end - start));
> >>>>>>> -------------
> >>>>>>>
> >>>>>>> the first loop, give time for jvm to warm up
> >>>>>>>
> >>>>>>> Below data compare the two implementations:
> >>>>>>>
> >>>>>>> before the patch:
> >>>>>>> Invoke Date.toString() 5000 times, cost: 6757
> >>>>>>> Invoke Time.toString() 5000 times, cost: 7699
> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
> >>>>>>>
> >>>>>>> after the patch:
> >>>>>>> Invoke Date.toString() 5000 times, cost: 84
> >>>>>>> Invoke Time.toString() 5000 times, cost: 95
> >>>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
> >>>>>>>
> >>>>>>>
> >>>>>>> We can gain obvious improvement.
> >>>>>>>
> >>>>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
> >>>>>>>> Are there any associated test cases with this change? On a quick
> >>>>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
> >>>>>>>> not, we need to start requiring some test cases for "simple
> >>>>>>>> improvements" like this to continue functional correctness.
> >>>>>>>>
> >>>>>>>> -Nathan
> >>>>>>>>
> >>>>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
> >>>>>>>>> Author: qiuxx
> >>>>>>>>> Date: Tue Aug 26 01:59:50 2008
> >>>>>>>>> New Revision: 689001
> >>>>>>>>>
> >>>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
> >>>>>>>>> Log:
> >>>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
> >>>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>>
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -17,8 +17,6 @@
> >>>>>>>>>
> >>>>>>>>> package java.sql;
> >>>>>>>>>
> >>>>>>>>> -import java.text.SimpleDateFormat;
> >>>>>>>>> -
> >>>>>>>>> /**
> >>>>>>>>> * A Date class which can consume and produce dates in SQL Date format.
> >>>>>>>>> * <p>
> >>>>>>>>> @@ -175,8 +173,28 @@
> >>>>>>>>> */
> >>>>>>>>> @Override
> >>>>>>>>> public String toString() {
> >>>>>>>>> - SimpleDateFormat dateFormat = new
> >>>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
> >>>>>>>>> - return dateFormat.format(this);
> >>>>>>>>> + StringBuilder sb = new StringBuilder(10);
> >>>>>>>>> +
> >>>>>>>>> + format((getYear() + 1900), 4, sb);
> >>>>>>>>> + sb.append('-');
> >>>>>>>>> + format((getMonth() + 1), 2, sb);
> >>>>>>>>> + sb.append('-');
> >>>>>>>>> + format(getDate(), 2, sb);
> >>>>>>>>> +
> >>>>>>>>> + return sb.toString();
> >>>>>>>>> + }
> >>>>>>>>> +
> >>>>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
> >>>>>>>>> +
> >>>>>>>>> + /*
> >>>>>>>>> + * Private method to format the time
> >>>>>>>>> + */
> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
> >>>>>>>>> + String str = String.valueOf(date);
> >>>>>>>>> + if (digits - str.length() > 0) {
> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
> >>>>>>>>> + }
> >>>>>>>>> + sb.append(str);
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> /**
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -17,7 +17,6 @@
> >>>>>>>>>
> >>>>>>>>> package java.sql;
> >>>>>>>>>
> >>>>>>>>> -import java.text.SimpleDateFormat;
> >>>>>>>>> import java.util.Date;
> >>>>>>>>>
> >>>>>>>>> /**
> >>>>>>>>> @@ -180,8 +179,28 @@
> >>>>>>>>> */
> >>>>>>>>> @Override
> >>>>>>>>> public String toString() {
> >>>>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
> >>>>>>>>> //$NON-NLS-1$
> >>>>>>>>> - return dateFormat.format(this);
> >>>>>>>>> + StringBuilder sb = new StringBuilder(8);
> >>>>>>>>> +
> >>>>>>>>> + format(getHours(), 2, sb);
> >>>>>>>>> + sb.append(':');
> >>>>>>>>> + format(getMinutes(), 2, sb);
> >>>>>>>>> + sb.append(':');
> >>>>>>>>> + format(getSeconds(), 2, sb);
> >>>>>>>>> +
> >>>>>>>>> + return sb.toString();
> >>>>>>>>> + }
> >>>>>>>>> +
> >>>>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
> >>>>>>>>> +
> >>>>>>>>> + /*
> >>>>>>>>> + * Private method to format the time
> >>>>>>>>> + */
> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
> >>>>>>>>> + String str = String.valueOf(date);
> >>>>>>>>> + if (digits - str.length() > 0) {
> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
> >>>>>>>>> + }
> >>>>>>>>> + sb.append(str);
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> /**
> >>>>>>>>>
> >>>>>>>>> Modified:
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> URL:
> >>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
> >>>>>>>>> ==============================================================================
> >>>>>>>>> ---
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> (original)
> >>>>>>>>> +++
> >>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> >>>>>>>>> Tue Aug 26 01:59:50 2008
> >>>>>>>>> @@ -309,62 +309,43 @@
> >>>>>>>>> @SuppressWarnings("deprecation")
> >>>>>>>>> @Override
> >>>>>>>>> public String toString() {
> >>>>>>>>> - /*
> >>>>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
> >>>>>>>>> simple
> >>>>>>>>> - * string of 9 integers, with leading Zeros
> >>>>>>>>> - */
> >>>>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
> >>>>>>>>> //$NON-NLS-1$
> >>>>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
> >>>>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
> >>>>>>>>> - String theNanos = decimalFormat.format(nanos);
> >>>>>>>>> - theNanos = stripTrailingZeros(theNanos);
> >>>>>>>>> -
> >>>>>>>>> - String year = format((getYear() + 1900), 4);
> >>>>>>>>> - String month = format((getMonth() + 1), 2);
> >>>>>>>>> - String date = format(getDate(), 2);
> >>>>>>>>> - String hours = format(getHours(), 2);
> >>>>>>>>> - String minutes = format(getMinutes(), 2);
> >>>>>>>>> - String seconds = format(getSeconds(), 2);
> >>>>>>>>> -
> >>>>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
> >>>>>>>>> minutes
> >>>>>>>>> - + ':' + seconds + '.' + theNanos;
> >>>>>>>>> - }
> >>>>>>>>> + StringBuilder sb = new StringBuilder(29);
> >>>>>>>>>
> >>>>>>>>> - /*
> >>>>>>>>> - * Private method to format the time
> >>>>>>>>> - */
> >>>>>>>>> - private String format(int date, int digits) {
> >>>>>>>>> - StringBuilder dateStringBuffer = new
> >>>>>>>>> StringBuilder(String.valueOf(date));
> >>>>>>>>> - while (dateStringBuffer.length() < digits) {
> >>>>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
> >>>>>>>>> + format((getYear() + 1900), 4, sb);
> >>>>>>>>> + sb.append('-');
> >>>>>>>>> + format((getMonth() + 1), 2, sb);
> >>>>>>>>> + sb.append('-');
> >>>>>>>>> + format(getDate(), 2, sb);
> >>>>>>>>> + sb.append(' ');
> >>>>>>>>> + format(getHours(), 2, sb);
> >>>>>>>>> + sb.append(':');
> >>>>>>>>> + format(getMinutes(), 2, sb);
> >>>>>>>>> + sb.append(':');
> >>>>>>>>> + format(getSeconds(), 2, sb);
> >>>>>>>>> + sb.append('.');
> >>>>>>>>> + if (nanos == 0) {
> >>>>>>>>> + sb.append('0');
> >>>>>>>>> + } else {
> >>>>>>>>> + format(nanos, 9, sb);
> >>>>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
> >>>>>>>>> + sb.setLength(sb.length() - 1);
> >>>>>>>>> + }
> >>>>>>>>> }
> >>>>>>>>> - return dateStringBuffer.toString();
> >>>>>>>>> +
> >>>>>>>>> + return sb.toString();
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> - /*
> >>>>>>>>> - * Private method to strip trailing '0' characters from a string.
> >>>>>>>>> @param
> >>>>>>>>> - * inputString the starting string @return a string with the
> >>>>>>>>> trailing zeros
> >>>>>>>>> - * stripped - will leave a single 0 at the beginning of the string
> >>>>>>>>> - */
> >>>>>>>>> - private String stripTrailingZeros(String inputString) {
> >>>>>>>>> - String finalString;
> >>>>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
> >>>>>>>>>
> >>>>>>>>> - int i;
> >>>>>>>>> - for (i = inputString.length(); i > 0; i--) {
> >>>>>>>>> - if (inputString.charAt(i - 1) != '0') {
> >>>>>>>>> - break;
> >>>>>>>>> - }
> >>>>>>>>> - /*
> >>>>>>>>> - * If the string has a 0 as its first character, return a
> >>>>>>>>> string
> >>>>>>>>> - * with a single '0'
> >>>>>>>>> - */
> >>>>>>>>> - if (i == 1) {
> >>>>>>>>> - return "0"; //$NON-NLS-1$
> >>>>>>>>> - }
> >>>>>>>>> + /*
> >>>>>>>>> + * Private method to format the time
> >>>>>>>>> + */
> >>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
> >>>>>>>>> + String str = String.valueOf(date);
> >>>>>>>>> + if (digits - str.length() > 0) {
> >>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
> >>>>>>>>> }
> >>>>>>>>> -
> >>>>>>>>> - finalString = inputString.substring(0, i);
> >>>>>>>>> - return finalString;
> >>>>>>>>> + sb.append(str);
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> /**
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Best Regards
> >>>>>>> Sean, Xiao Xia Qiu
> >>>>>>>
> >>>>>>> China Software Development Lab, IBM
> >>>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Sent from Gmail for mobile | mobile.google.com
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Best Regards
> >>>>> Sean, Xiao Xia Qiu
> >>>>>
> >>>>> China Software Development Lab, IBM
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Tony Wu
> >>> China Software Development Lab, IBM
> >>>
> >>
> >
> >
> >
> > --
> > Tony Wu
> > China Software Development Lab, IBM
> >
>
--
Tony Wu
China Software Development Lab, IBM
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nd...@apache.org>.
Since the classes are supposed to be TimeZone-independant, couldn't
all of the TimeZone code be removed?
-Nathan
On Mon, Sep 1, 2008 at 12:19 AM, Tony Wu <wu...@gmail.com> wrote:
> I reviewed these 3 tests. It's true that locales other than GMT have
> been covered in the TimeTest and TimeStampTest but not in DateTest.
> And the comments in the testToString is confusing ...
> Also some testcases in TimestampTest have dependency to each other.
> Since other locales have been covered by testToString, I just
> explicity set the default locale to GMT at the beginning of these
> testcases.
>
> I've corrected them at r690847.
>
> On Sat, Aug 30, 2008 at 10:22 AM, Nathan Beyer <nd...@apache.org> wrote:
>> But, the tests are invalid, correct? At a minimum, all of the TimeZone
>> code in the tests is unnecessary.
>>
>> I'm not trying to insinuate or infer that the patch is wrong. I just
>> like to look at each code change in a larger context. In this case,
>> the tests are still passing, but upon closer inspection of the tests,
>> they seem to be invalid or, as mentioned above, they have unnecessary
>> comments and seemingly invalid comments.
>>
>> -Nathan
>>
>> On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wu...@gmail.com> wrote:
>>> I think so. The format has been explicitly specified by spec, such as
>>> yyyy-mm-dd hh:mm:ss.fffffffff
>>>
>>> Thus it should be locale independent and this patch is valid.
>>>
>>> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
>>>> I looked at the test in further detail and they seem okay. The tests
>>>> seem odd though, as they set TimeZone values and mention things like
>>>> TimeZone affecting the output. That shouldn't be the case though,
>>>> correct? The formats are all in UTC/GMT, correct?
>>>>
>>>> -Nathan
>>>>
>>>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
>>>>> As Regis said, there exist tests for these toString method, which,
>>>>> IMHO, are covering this modfication.
>>>>> Shall we add more tests?
>>>>>
>>>>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>>>>>> I was referring to a functional test, not a performance test.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>>>>>>> Here is a small test from Regis:
>>>>>>>
>>>>>>> ----------
>>>>>>> int count = 5000;
>>>>>>>
>>>>>>> for (int i = 0; i < count; ++i) { new Date(new
>>>>>>> java.util.Date().getTime()).toString(); }
>>>>>>>
>>>>>>> Date date = new Date(new java.util.Date().getTime());
>>>>>>> long start = System.currentTimeMillis();
>>>>>>>
>>>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
>>>>>>> long end = System.currentTimeMillis();
>>>>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>>>>>>> " + (end - start));
>>>>>>>
>>>>>>> Time time = new Time(new java.util.Date().getTime());
>>>>>>> start = System.currentTimeMillis();
>>>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
>>>>>>> end = System.currentTimeMillis();
>>>>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>>>>>>> " + (end - start));
>>>>>>>
>>>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>>>>>>> start = System.currentTimeMillis();
>>>>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>>>>>>> end = System.currentTimeMillis();
>>>>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>>>>>>> cost: " + (end - start));
>>>>>>> -------------
>>>>>>>
>>>>>>> the first loop, give time for jvm to warm up
>>>>>>>
>>>>>>> Below data compare the two implementations:
>>>>>>>
>>>>>>> before the patch:
>>>>>>> Invoke Date.toString() 5000 times, cost: 6757
>>>>>>> Invoke Time.toString() 5000 times, cost: 7699
>>>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>>>>>>
>>>>>>> after the patch:
>>>>>>> Invoke Date.toString() 5000 times, cost: 84
>>>>>>> Invoke Time.toString() 5000 times, cost: 95
>>>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
>>>>>>>
>>>>>>>
>>>>>>> We can gain obvious improvement.
>>>>>>>
>>>>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>>>>>>> Are there any associated test cases with this change? On a quick
>>>>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>>>>>>> not, we need to start requiring some test cases for "simple
>>>>>>>> improvements" like this to continue functional correctness.
>>>>>>>>
>>>>>>>> -Nathan
>>>>>>>>
>>>>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>>>>>>> Author: qiuxx
>>>>>>>>> Date: Tue Aug 26 01:59:50 2008
>>>>>>>>> New Revision: 689001
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>>>>>>> Log:
>>>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>>
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>>>
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>>>
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>>> URL:
>>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> ---
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>>> (original)
>>>>>>>>> +++
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>>> @@ -17,8 +17,6 @@
>>>>>>>>>
>>>>>>>>> package java.sql;
>>>>>>>>>
>>>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>>>> -
>>>>>>>>> /**
>>>>>>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>>>>>>> * <p>
>>>>>>>>> @@ -175,8 +173,28 @@
>>>>>>>>> */
>>>>>>>>> @Override
>>>>>>>>> public String toString() {
>>>>>>>>> - SimpleDateFormat dateFormat = new
>>>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>>>>>>> - return dateFormat.format(this);
>>>>>>>>> + StringBuilder sb = new StringBuilder(10);
>>>>>>>>> +
>>>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>>>> + sb.append('-');
>>>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>>>> + sb.append('-');
>>>>>>>>> + format(getDate(), 2, sb);
>>>>>>>>> +
>>>>>>>>> + return sb.toString();
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * Private method to format the time
>>>>>>>>> + */
>>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>>> + String str = String.valueOf(date);
>>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>>> + }
>>>>>>>>> + sb.append(str);
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>>> URL:
>>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> ---
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>>> (original)
>>>>>>>>> +++
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>>> @@ -17,7 +17,6 @@
>>>>>>>>>
>>>>>>>>> package java.sql;
>>>>>>>>>
>>>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>>>> import java.util.Date;
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>> @@ -180,8 +179,28 @@
>>>>>>>>> */
>>>>>>>>> @Override
>>>>>>>>> public String toString() {
>>>>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>>>>>>> //$NON-NLS-1$
>>>>>>>>> - return dateFormat.format(this);
>>>>>>>>> + StringBuilder sb = new StringBuilder(8);
>>>>>>>>> +
>>>>>>>>> + format(getHours(), 2, sb);
>>>>>>>>> + sb.append(':');
>>>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>>>> + sb.append(':');
>>>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>>>> +
>>>>>>>>> + return sb.toString();
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * Private method to format the time
>>>>>>>>> + */
>>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>>> + String str = String.valueOf(date);
>>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>>> + }
>>>>>>>>> + sb.append(str);
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>>> URL:
>>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> ---
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>>> (original)
>>>>>>>>> +++
>>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>>> @@ -309,62 +309,43 @@
>>>>>>>>> @SuppressWarnings("deprecation")
>>>>>>>>> @Override
>>>>>>>>> public String toString() {
>>>>>>>>> - /*
>>>>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>>>>>>> simple
>>>>>>>>> - * string of 9 integers, with leading Zeros
>>>>>>>>> - */
>>>>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>>>>>>> //$NON-NLS-1$
>>>>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>>>>>>> - String theNanos = decimalFormat.format(nanos);
>>>>>>>>> - theNanos = stripTrailingZeros(theNanos);
>>>>>>>>> -
>>>>>>>>> - String year = format((getYear() + 1900), 4);
>>>>>>>>> - String month = format((getMonth() + 1), 2);
>>>>>>>>> - String date = format(getDate(), 2);
>>>>>>>>> - String hours = format(getHours(), 2);
>>>>>>>>> - String minutes = format(getMinutes(), 2);
>>>>>>>>> - String seconds = format(getSeconds(), 2);
>>>>>>>>> -
>>>>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>>>>>>> minutes
>>>>>>>>> - + ':' + seconds + '.' + theNanos;
>>>>>>>>> - }
>>>>>>>>> + StringBuilder sb = new StringBuilder(29);
>>>>>>>>>
>>>>>>>>> - /*
>>>>>>>>> - * Private method to format the time
>>>>>>>>> - */
>>>>>>>>> - private String format(int date, int digits) {
>>>>>>>>> - StringBuilder dateStringBuffer = new
>>>>>>>>> StringBuilder(String.valueOf(date));
>>>>>>>>> - while (dateStringBuffer.length() < digits) {
>>>>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>>>> + sb.append('-');
>>>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>>>> + sb.append('-');
>>>>>>>>> + format(getDate(), 2, sb);
>>>>>>>>> + sb.append(' ');
>>>>>>>>> + format(getHours(), 2, sb);
>>>>>>>>> + sb.append(':');
>>>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>>>> + sb.append(':');
>>>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>>>> + sb.append('.');
>>>>>>>>> + if (nanos == 0) {
>>>>>>>>> + sb.append('0');
>>>>>>>>> + } else {
>>>>>>>>> + format(nanos, 9, sb);
>>>>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>>>>>>> + sb.setLength(sb.length() - 1);
>>>>>>>>> + }
>>>>>>>>> }
>>>>>>>>> - return dateStringBuffer.toString();
>>>>>>>>> +
>>>>>>>>> + return sb.toString();
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> - /*
>>>>>>>>> - * Private method to strip trailing '0' characters from a string.
>>>>>>>>> @param
>>>>>>>>> - * inputString the starting string @return a string with the
>>>>>>>>> trailing zeros
>>>>>>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>>>>>>> - */
>>>>>>>>> - private String stripTrailingZeros(String inputString) {
>>>>>>>>> - String finalString;
>>>>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>>>>>>
>>>>>>>>> - int i;
>>>>>>>>> - for (i = inputString.length(); i > 0; i--) {
>>>>>>>>> - if (inputString.charAt(i - 1) != '0') {
>>>>>>>>> - break;
>>>>>>>>> - }
>>>>>>>>> - /*
>>>>>>>>> - * If the string has a 0 as its first character, return a
>>>>>>>>> string
>>>>>>>>> - * with a single '0'
>>>>>>>>> - */
>>>>>>>>> - if (i == 1) {
>>>>>>>>> - return "0"; //$NON-NLS-1$
>>>>>>>>> - }
>>>>>>>>> + /*
>>>>>>>>> + * Private method to format the time
>>>>>>>>> + */
>>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>>> + String str = String.valueOf(date);
>>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>>> }
>>>>>>>>> -
>>>>>>>>> - finalString = inputString.substring(0, i);
>>>>>>>>> - return finalString;
>>>>>>>>> + sb.append(str);
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best Regards
>>>>>>> Sean, Xiao Xia Qiu
>>>>>>>
>>>>>>> China Software Development Lab, IBM
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Sent from Gmail for mobile | mobile.google.com
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best Regards
>>>>> Sean, Xiao Xia Qiu
>>>>>
>>>>> China Software Development Lab, IBM
>>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Tony Wu
>>> China Software Development Lab, IBM
>>>
>>
>
>
>
> --
> Tony Wu
> China Software Development Lab, IBM
>
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Tony Wu <wu...@gmail.com>.
I reviewed these 3 tests. It's true that locales other than GMT have
been covered in the TimeTest and TimeStampTest but not in DateTest.
And the comments in the testToString is confusing ...
Also some testcases in TimestampTest have dependency to each other.
Since other locales have been covered by testToString, I just
explicity set the default locale to GMT at the beginning of these
testcases.
I've corrected them at r690847.
On Sat, Aug 30, 2008 at 10:22 AM, Nathan Beyer <nd...@apache.org> wrote:
> But, the tests are invalid, correct? At a minimum, all of the TimeZone
> code in the tests is unnecessary.
>
> I'm not trying to insinuate or infer that the patch is wrong. I just
> like to look at each code change in a larger context. In this case,
> the tests are still passing, but upon closer inspection of the tests,
> they seem to be invalid or, as mentioned above, they have unnecessary
> comments and seemingly invalid comments.
>
> -Nathan
>
> On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wu...@gmail.com> wrote:
>> I think so. The format has been explicitly specified by spec, such as
>> yyyy-mm-dd hh:mm:ss.fffffffff
>>
>> Thus it should be locale independent and this patch is valid.
>>
>> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
>>> I looked at the test in further detail and they seem okay. The tests
>>> seem odd though, as they set TimeZone values and mention things like
>>> TimeZone affecting the output. That shouldn't be the case though,
>>> correct? The formats are all in UTC/GMT, correct?
>>>
>>> -Nathan
>>>
>>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
>>>> As Regis said, there exist tests for these toString method, which,
>>>> IMHO, are covering this modfication.
>>>> Shall we add more tests?
>>>>
>>>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>>>>> I was referring to a functional test, not a performance test.
>>>>>
>>>>>
>>>>>
>>>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>>>>>> Here is a small test from Regis:
>>>>>>
>>>>>> ----------
>>>>>> int count = 5000;
>>>>>>
>>>>>> for (int i = 0; i < count; ++i) { new Date(new
>>>>>> java.util.Date().getTime()).toString(); }
>>>>>>
>>>>>> Date date = new Date(new java.util.Date().getTime());
>>>>>> long start = System.currentTimeMillis();
>>>>>>
>>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
>>>>>> long end = System.currentTimeMillis();
>>>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>>>>>> " + (end - start));
>>>>>>
>>>>>> Time time = new Time(new java.util.Date().getTime());
>>>>>> start = System.currentTimeMillis();
>>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
>>>>>> end = System.currentTimeMillis();
>>>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>>>>>> " + (end - start));
>>>>>>
>>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>>>>>> start = System.currentTimeMillis();
>>>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>>>>>> end = System.currentTimeMillis();
>>>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>>>>>> cost: " + (end - start));
>>>>>> -------------
>>>>>>
>>>>>> the first loop, give time for jvm to warm up
>>>>>>
>>>>>> Below data compare the two implementations:
>>>>>>
>>>>>> before the patch:
>>>>>> Invoke Date.toString() 5000 times, cost: 6757
>>>>>> Invoke Time.toString() 5000 times, cost: 7699
>>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>>>>>
>>>>>> after the patch:
>>>>>> Invoke Date.toString() 5000 times, cost: 84
>>>>>> Invoke Time.toString() 5000 times, cost: 95
>>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
>>>>>>
>>>>>>
>>>>>> We can gain obvious improvement.
>>>>>>
>>>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>>>>>> Are there any associated test cases with this change? On a quick
>>>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>>>>>> not, we need to start requiring some test cases for "simple
>>>>>>> improvements" like this to continue functional correctness.
>>>>>>>
>>>>>>> -Nathan
>>>>>>>
>>>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>>>>>> Author: qiuxx
>>>>>>>> Date: Tue Aug 26 01:59:50 2008
>>>>>>>> New Revision: 689001
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>>>>>> Log:
>>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>>
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>>
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>>
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> ---
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>> (original)
>>>>>>>> +++
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>> @@ -17,8 +17,6 @@
>>>>>>>>
>>>>>>>> package java.sql;
>>>>>>>>
>>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>>> -
>>>>>>>> /**
>>>>>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>>>>>> * <p>
>>>>>>>> @@ -175,8 +173,28 @@
>>>>>>>> */
>>>>>>>> @Override
>>>>>>>> public String toString() {
>>>>>>>> - SimpleDateFormat dateFormat = new
>>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>>>>>> - return dateFormat.format(this);
>>>>>>>> + StringBuilder sb = new StringBuilder(10);
>>>>>>>> +
>>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>>> + sb.append('-');
>>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>>> + sb.append('-');
>>>>>>>> + format(getDate(), 2, sb);
>>>>>>>> +
>>>>>>>> + return sb.toString();
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Private method to format the time
>>>>>>>> + */
>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>> + String str = String.valueOf(date);
>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>> + }
>>>>>>>> + sb.append(str);
>>>>>>>> }
>>>>>>>>
>>>>>>>> /**
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> ---
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>> (original)
>>>>>>>> +++
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>> @@ -17,7 +17,6 @@
>>>>>>>>
>>>>>>>> package java.sql;
>>>>>>>>
>>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>>> import java.util.Date;
>>>>>>>>
>>>>>>>> /**
>>>>>>>> @@ -180,8 +179,28 @@
>>>>>>>> */
>>>>>>>> @Override
>>>>>>>> public String toString() {
>>>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>>>>>> //$NON-NLS-1$
>>>>>>>> - return dateFormat.format(this);
>>>>>>>> + StringBuilder sb = new StringBuilder(8);
>>>>>>>> +
>>>>>>>> + format(getHours(), 2, sb);
>>>>>>>> + sb.append(':');
>>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>>> + sb.append(':');
>>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>>> +
>>>>>>>> + return sb.toString();
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Private method to format the time
>>>>>>>> + */
>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>> + String str = String.valueOf(date);
>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>> + }
>>>>>>>> + sb.append(str);
>>>>>>>> }
>>>>>>>>
>>>>>>>> /**
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> ---
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>> (original)
>>>>>>>> +++
>>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>>> @@ -309,62 +309,43 @@
>>>>>>>> @SuppressWarnings("deprecation")
>>>>>>>> @Override
>>>>>>>> public String toString() {
>>>>>>>> - /*
>>>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>>>>>> simple
>>>>>>>> - * string of 9 integers, with leading Zeros
>>>>>>>> - */
>>>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>>>>>> //$NON-NLS-1$
>>>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>>>>>> - String theNanos = decimalFormat.format(nanos);
>>>>>>>> - theNanos = stripTrailingZeros(theNanos);
>>>>>>>> -
>>>>>>>> - String year = format((getYear() + 1900), 4);
>>>>>>>> - String month = format((getMonth() + 1), 2);
>>>>>>>> - String date = format(getDate(), 2);
>>>>>>>> - String hours = format(getHours(), 2);
>>>>>>>> - String minutes = format(getMinutes(), 2);
>>>>>>>> - String seconds = format(getSeconds(), 2);
>>>>>>>> -
>>>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>>>>>> minutes
>>>>>>>> - + ':' + seconds + '.' + theNanos;
>>>>>>>> - }
>>>>>>>> + StringBuilder sb = new StringBuilder(29);
>>>>>>>>
>>>>>>>> - /*
>>>>>>>> - * Private method to format the time
>>>>>>>> - */
>>>>>>>> - private String format(int date, int digits) {
>>>>>>>> - StringBuilder dateStringBuffer = new
>>>>>>>> StringBuilder(String.valueOf(date));
>>>>>>>> - while (dateStringBuffer.length() < digits) {
>>>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>>> + sb.append('-');
>>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>>> + sb.append('-');
>>>>>>>> + format(getDate(), 2, sb);
>>>>>>>> + sb.append(' ');
>>>>>>>> + format(getHours(), 2, sb);
>>>>>>>> + sb.append(':');
>>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>>> + sb.append(':');
>>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>>> + sb.append('.');
>>>>>>>> + if (nanos == 0) {
>>>>>>>> + sb.append('0');
>>>>>>>> + } else {
>>>>>>>> + format(nanos, 9, sb);
>>>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>>>>>> + sb.setLength(sb.length() - 1);
>>>>>>>> + }
>>>>>>>> }
>>>>>>>> - return dateStringBuffer.toString();
>>>>>>>> +
>>>>>>>> + return sb.toString();
>>>>>>>> }
>>>>>>>>
>>>>>>>> - /*
>>>>>>>> - * Private method to strip trailing '0' characters from a string.
>>>>>>>> @param
>>>>>>>> - * inputString the starting string @return a string with the
>>>>>>>> trailing zeros
>>>>>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>>>>>> - */
>>>>>>>> - private String stripTrailingZeros(String inputString) {
>>>>>>>> - String finalString;
>>>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>>>>>
>>>>>>>> - int i;
>>>>>>>> - for (i = inputString.length(); i > 0; i--) {
>>>>>>>> - if (inputString.charAt(i - 1) != '0') {
>>>>>>>> - break;
>>>>>>>> - }
>>>>>>>> - /*
>>>>>>>> - * If the string has a 0 as its first character, return a
>>>>>>>> string
>>>>>>>> - * with a single '0'
>>>>>>>> - */
>>>>>>>> - if (i == 1) {
>>>>>>>> - return "0"; //$NON-NLS-1$
>>>>>>>> - }
>>>>>>>> + /*
>>>>>>>> + * Private method to format the time
>>>>>>>> + */
>>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>>> + String str = String.valueOf(date);
>>>>>>>> + if (digits - str.length() > 0) {
>>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>>> }
>>>>>>>> -
>>>>>>>> - finalString = inputString.substring(0, i);
>>>>>>>> - return finalString;
>>>>>>>> + sb.append(str);
>>>>>>>> }
>>>>>>>>
>>>>>>>> /**
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best Regards
>>>>>> Sean, Xiao Xia Qiu
>>>>>>
>>>>>> China Software Development Lab, IBM
>>>>>>
>>>>>
>>>>> --
>>>>> Sent from Gmail for mobile | mobile.google.com
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Best Regards
>>>> Sean, Xiao Xia Qiu
>>>>
>>>> China Software Development Lab, IBM
>>>>
>>>
>>
>>
>>
>> --
>> Tony Wu
>> China Software Development Lab, IBM
>>
>
--
Tony Wu
China Software Development Lab, IBM
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nd...@apache.org>.
But, the tests are invalid, correct? At a minimum, all of the TimeZone
code in the tests is unnecessary.
I'm not trying to insinuate or infer that the patch is wrong. I just
like to look at each code change in a larger context. In this case,
the tests are still passing, but upon closer inspection of the tests,
they seem to be invalid or, as mentioned above, they have unnecessary
comments and seemingly invalid comments.
-Nathan
On Thu, Aug 28, 2008 at 9:46 PM, Tony Wu <wu...@gmail.com> wrote:
> I think so. The format has been explicitly specified by spec, such as
> yyyy-mm-dd hh:mm:ss.fffffffff
>
> Thus it should be locale independent and this patch is valid.
>
> On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
>> I looked at the test in further detail and they seem okay. The tests
>> seem odd though, as they set TimeZone values and mention things like
>> TimeZone affecting the output. That shouldn't be the case though,
>> correct? The formats are all in UTC/GMT, correct?
>>
>> -Nathan
>>
>> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
>>> As Regis said, there exist tests for these toString method, which,
>>> IMHO, are covering this modfication.
>>> Shall we add more tests?
>>>
>>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>>>> I was referring to a functional test, not a performance test.
>>>>
>>>>
>>>>
>>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>>>>> Here is a small test from Regis:
>>>>>
>>>>> ----------
>>>>> int count = 5000;
>>>>>
>>>>> for (int i = 0; i < count; ++i) { new Date(new
>>>>> java.util.Date().getTime()).toString(); }
>>>>>
>>>>> Date date = new Date(new java.util.Date().getTime());
>>>>> long start = System.currentTimeMillis();
>>>>>
>>>>> for (int i = 0; i < count; ++i) { date.toString(); }
>>>>> long end = System.currentTimeMillis();
>>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>>>>> " + (end - start));
>>>>>
>>>>> Time time = new Time(new java.util.Date().getTime());
>>>>> start = System.currentTimeMillis();
>>>>> for (int i = 0; i < count; ++i) { time.toString(); }
>>>>> end = System.currentTimeMillis();
>>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>>>>> " + (end - start));
>>>>>
>>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>>>>> start = System.currentTimeMillis();
>>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>>>>> end = System.currentTimeMillis();
>>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>>>>> cost: " + (end - start));
>>>>> -------------
>>>>>
>>>>> the first loop, give time for jvm to warm up
>>>>>
>>>>> Below data compare the two implementations:
>>>>>
>>>>> before the patch:
>>>>> Invoke Date.toString() 5000 times, cost: 6757
>>>>> Invoke Time.toString() 5000 times, cost: 7699
>>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>>>>
>>>>> after the patch:
>>>>> Invoke Date.toString() 5000 times, cost: 84
>>>>> Invoke Time.toString() 5000 times, cost: 95
>>>>> Invoke Timestamp.toString() 5000 times, cost: 272
>>>>>
>>>>>
>>>>> We can gain obvious improvement.
>>>>>
>>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>>>>> Are there any associated test cases with this change? On a quick
>>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>>>>> not, we need to start requiring some test cases for "simple
>>>>>> improvements" like this to continue functional correctness.
>>>>>>
>>>>>> -Nathan
>>>>>>
>>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>>>>> Author: qiuxx
>>>>>>> Date: Tue Aug 26 01:59:50 2008
>>>>>>> New Revision: 689001
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>>>>> Log:
>>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>>>>
>>>>>>> Modified:
>>>>>>>
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>>
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>>
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>>
>>>>>>> Modified:
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>> ==============================================================================
>>>>>>> ---
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>> (original)
>>>>>>> +++
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>> @@ -17,8 +17,6 @@
>>>>>>>
>>>>>>> package java.sql;
>>>>>>>
>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>> -
>>>>>>> /**
>>>>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>>>>> * <p>
>>>>>>> @@ -175,8 +173,28 @@
>>>>>>> */
>>>>>>> @Override
>>>>>>> public String toString() {
>>>>>>> - SimpleDateFormat dateFormat = new
>>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>>>>> - return dateFormat.format(this);
>>>>>>> + StringBuilder sb = new StringBuilder(10);
>>>>>>> +
>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>> + sb.append('-');
>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>> + sb.append('-');
>>>>>>> + format(getDate(), 2, sb);
>>>>>>> +
>>>>>>> + return sb.toString();
>>>>>>> + }
>>>>>>> +
>>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Private method to format the time
>>>>>>> + */
>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>> + String str = String.valueOf(date);
>>>>>>> + if (digits - str.length() > 0) {
>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>> + }
>>>>>>> + sb.append(str);
>>>>>>> }
>>>>>>>
>>>>>>> /**
>>>>>>>
>>>>>>> Modified:
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>> ==============================================================================
>>>>>>> ---
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>> (original)
>>>>>>> +++
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>> @@ -17,7 +17,6 @@
>>>>>>>
>>>>>>> package java.sql;
>>>>>>>
>>>>>>> -import java.text.SimpleDateFormat;
>>>>>>> import java.util.Date;
>>>>>>>
>>>>>>> /**
>>>>>>> @@ -180,8 +179,28 @@
>>>>>>> */
>>>>>>> @Override
>>>>>>> public String toString() {
>>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>>>>> //$NON-NLS-1$
>>>>>>> - return dateFormat.format(this);
>>>>>>> + StringBuilder sb = new StringBuilder(8);
>>>>>>> +
>>>>>>> + format(getHours(), 2, sb);
>>>>>>> + sb.append(':');
>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>> + sb.append(':');
>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>> +
>>>>>>> + return sb.toString();
>>>>>>> + }
>>>>>>> +
>>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Private method to format the time
>>>>>>> + */
>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>> + String str = String.valueOf(date);
>>>>>>> + if (digits - str.length() > 0) {
>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>> + }
>>>>>>> + sb.append(str);
>>>>>>> }
>>>>>>>
>>>>>>> /**
>>>>>>>
>>>>>>> Modified:
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>>> ==============================================================================
>>>>>>> ---
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>> (original)
>>>>>>> +++
>>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>> Tue Aug 26 01:59:50 2008
>>>>>>> @@ -309,62 +309,43 @@
>>>>>>> @SuppressWarnings("deprecation")
>>>>>>> @Override
>>>>>>> public String toString() {
>>>>>>> - /*
>>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>>>>> simple
>>>>>>> - * string of 9 integers, with leading Zeros
>>>>>>> - */
>>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>>>>> //$NON-NLS-1$
>>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>>>>> - String theNanos = decimalFormat.format(nanos);
>>>>>>> - theNanos = stripTrailingZeros(theNanos);
>>>>>>> -
>>>>>>> - String year = format((getYear() + 1900), 4);
>>>>>>> - String month = format((getMonth() + 1), 2);
>>>>>>> - String date = format(getDate(), 2);
>>>>>>> - String hours = format(getHours(), 2);
>>>>>>> - String minutes = format(getMinutes(), 2);
>>>>>>> - String seconds = format(getSeconds(), 2);
>>>>>>> -
>>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>>>>> minutes
>>>>>>> - + ':' + seconds + '.' + theNanos;
>>>>>>> - }
>>>>>>> + StringBuilder sb = new StringBuilder(29);
>>>>>>>
>>>>>>> - /*
>>>>>>> - * Private method to format the time
>>>>>>> - */
>>>>>>> - private String format(int date, int digits) {
>>>>>>> - StringBuilder dateStringBuffer = new
>>>>>>> StringBuilder(String.valueOf(date));
>>>>>>> - while (dateStringBuffer.length() < digits) {
>>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>>> + sb.append('-');
>>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>>> + sb.append('-');
>>>>>>> + format(getDate(), 2, sb);
>>>>>>> + sb.append(' ');
>>>>>>> + format(getHours(), 2, sb);
>>>>>>> + sb.append(':');
>>>>>>> + format(getMinutes(), 2, sb);
>>>>>>> + sb.append(':');
>>>>>>> + format(getSeconds(), 2, sb);
>>>>>>> + sb.append('.');
>>>>>>> + if (nanos == 0) {
>>>>>>> + sb.append('0');
>>>>>>> + } else {
>>>>>>> + format(nanos, 9, sb);
>>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>>>>> + sb.setLength(sb.length() - 1);
>>>>>>> + }
>>>>>>> }
>>>>>>> - return dateStringBuffer.toString();
>>>>>>> +
>>>>>>> + return sb.toString();
>>>>>>> }
>>>>>>>
>>>>>>> - /*
>>>>>>> - * Private method to strip trailing '0' characters from a string.
>>>>>>> @param
>>>>>>> - * inputString the starting string @return a string with the
>>>>>>> trailing zeros
>>>>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>>>>> - */
>>>>>>> - private String stripTrailingZeros(String inputString) {
>>>>>>> - String finalString;
>>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>>>>
>>>>>>> - int i;
>>>>>>> - for (i = inputString.length(); i > 0; i--) {
>>>>>>> - if (inputString.charAt(i - 1) != '0') {
>>>>>>> - break;
>>>>>>> - }
>>>>>>> - /*
>>>>>>> - * If the string has a 0 as its first character, return a
>>>>>>> string
>>>>>>> - * with a single '0'
>>>>>>> - */
>>>>>>> - if (i == 1) {
>>>>>>> - return "0"; //$NON-NLS-1$
>>>>>>> - }
>>>>>>> + /*
>>>>>>> + * Private method to format the time
>>>>>>> + */
>>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>>> + String str = String.valueOf(date);
>>>>>>> + if (digits - str.length() > 0) {
>>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>>> }
>>>>>>> -
>>>>>>> - finalString = inputString.substring(0, i);
>>>>>>> - return finalString;
>>>>>>> + sb.append(str);
>>>>>>> }
>>>>>>>
>>>>>>> /**
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best Regards
>>>>> Sean, Xiao Xia Qiu
>>>>>
>>>>> China Software Development Lab, IBM
>>>>>
>>>>
>>>> --
>>>> Sent from Gmail for mobile | mobile.google.com
>>>>
>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Sean, Xiao Xia Qiu
>>>
>>> China Software Development Lab, IBM
>>>
>>
>
>
>
> --
> Tony Wu
> China Software Development Lab, IBM
>
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Tony Wu <wu...@gmail.com>.
I think so. The format has been explicitly specified by spec, such as
yyyy-mm-dd hh:mm:ss.fffffffff
Thus it should be locale independent and this patch is valid.
On Fri, Aug 29, 2008 at 12:17 AM, Nathan Beyer <nd...@apache.org> wrote:
> I looked at the test in further detail and they seem okay. The tests
> seem odd though, as they set TimeZone values and mention things like
> TimeZone affecting the output. That shouldn't be the case though,
> correct? The formats are all in UTC/GMT, correct?
>
> -Nathan
>
> On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
>> As Regis said, there exist tests for these toString method, which,
>> IMHO, are covering this modfication.
>> Shall we add more tests?
>>
>> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>>> I was referring to a functional test, not a performance test.
>>>
>>>
>>>
>>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>>>> Here is a small test from Regis:
>>>>
>>>> ----------
>>>> int count = 5000;
>>>>
>>>> for (int i = 0; i < count; ++i) { new Date(new
>>>> java.util.Date().getTime()).toString(); }
>>>>
>>>> Date date = new Date(new java.util.Date().getTime());
>>>> long start = System.currentTimeMillis();
>>>>
>>>> for (int i = 0; i < count; ++i) { date.toString(); }
>>>> long end = System.currentTimeMillis();
>>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>>>> " + (end - start));
>>>>
>>>> Time time = new Time(new java.util.Date().getTime());
>>>> start = System.currentTimeMillis();
>>>> for (int i = 0; i < count; ++i) { time.toString(); }
>>>> end = System.currentTimeMillis();
>>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>>>> " + (end - start));
>>>>
>>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>>>> start = System.currentTimeMillis();
>>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>>>> end = System.currentTimeMillis();
>>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>>>> cost: " + (end - start));
>>>> -------------
>>>>
>>>> the first loop, give time for jvm to warm up
>>>>
>>>> Below data compare the two implementations:
>>>>
>>>> before the patch:
>>>> Invoke Date.toString() 5000 times, cost: 6757
>>>> Invoke Time.toString() 5000 times, cost: 7699
>>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>>>
>>>> after the patch:
>>>> Invoke Date.toString() 5000 times, cost: 84
>>>> Invoke Time.toString() 5000 times, cost: 95
>>>> Invoke Timestamp.toString() 5000 times, cost: 272
>>>>
>>>>
>>>> We can gain obvious improvement.
>>>>
>>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>>>> Are there any associated test cases with this change? On a quick
>>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>>>> not, we need to start requiring some test cases for "simple
>>>>> improvements" like this to continue functional correctness.
>>>>>
>>>>> -Nathan
>>>>>
>>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>>>> Author: qiuxx
>>>>>> Date: Tue Aug 26 01:59:50 2008
>>>>>> New Revision: 689001
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>>>> Log:
>>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>>>
>>>>>> Modified:
>>>>>>
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>>
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>>
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>>
>>>>>> Modified:
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>> ==============================================================================
>>>>>> ---
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>> (original)
>>>>>> +++
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>> Tue Aug 26 01:59:50 2008
>>>>>> @@ -17,8 +17,6 @@
>>>>>>
>>>>>> package java.sql;
>>>>>>
>>>>>> -import java.text.SimpleDateFormat;
>>>>>> -
>>>>>> /**
>>>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>>>> * <p>
>>>>>> @@ -175,8 +173,28 @@
>>>>>> */
>>>>>> @Override
>>>>>> public String toString() {
>>>>>> - SimpleDateFormat dateFormat = new
>>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>>>> - return dateFormat.format(this);
>>>>>> + StringBuilder sb = new StringBuilder(10);
>>>>>> +
>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>> + sb.append('-');
>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>> + sb.append('-');
>>>>>> + format(getDate(), 2, sb);
>>>>>> +
>>>>>> + return sb.toString();
>>>>>> + }
>>>>>> +
>>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>>>> +
>>>>>> + /*
>>>>>> + * Private method to format the time
>>>>>> + */
>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>> + String str = String.valueOf(date);
>>>>>> + if (digits - str.length() > 0) {
>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>> + }
>>>>>> + sb.append(str);
>>>>>> }
>>>>>>
>>>>>> /**
>>>>>>
>>>>>> Modified:
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>> ==============================================================================
>>>>>> ---
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>> (original)
>>>>>> +++
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>> Tue Aug 26 01:59:50 2008
>>>>>> @@ -17,7 +17,6 @@
>>>>>>
>>>>>> package java.sql;
>>>>>>
>>>>>> -import java.text.SimpleDateFormat;
>>>>>> import java.util.Date;
>>>>>>
>>>>>> /**
>>>>>> @@ -180,8 +179,28 @@
>>>>>> */
>>>>>> @Override
>>>>>> public String toString() {
>>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>>>> //$NON-NLS-1$
>>>>>> - return dateFormat.format(this);
>>>>>> + StringBuilder sb = new StringBuilder(8);
>>>>>> +
>>>>>> + format(getHours(), 2, sb);
>>>>>> + sb.append(':');
>>>>>> + format(getMinutes(), 2, sb);
>>>>>> + sb.append(':');
>>>>>> + format(getSeconds(), 2, sb);
>>>>>> +
>>>>>> + return sb.toString();
>>>>>> + }
>>>>>> +
>>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>>>> +
>>>>>> + /*
>>>>>> + * Private method to format the time
>>>>>> + */
>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>> + String str = String.valueOf(date);
>>>>>> + if (digits - str.length() > 0) {
>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>> + }
>>>>>> + sb.append(str);
>>>>>> }
>>>>>>
>>>>>> /**
>>>>>>
>>>>>> Modified:
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>>> ==============================================================================
>>>>>> ---
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>> (original)
>>>>>> +++
>>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>> Tue Aug 26 01:59:50 2008
>>>>>> @@ -309,62 +309,43 @@
>>>>>> @SuppressWarnings("deprecation")
>>>>>> @Override
>>>>>> public String toString() {
>>>>>> - /*
>>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>>>> simple
>>>>>> - * string of 9 integers, with leading Zeros
>>>>>> - */
>>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>>>> //$NON-NLS-1$
>>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>>>> - String theNanos = decimalFormat.format(nanos);
>>>>>> - theNanos = stripTrailingZeros(theNanos);
>>>>>> -
>>>>>> - String year = format((getYear() + 1900), 4);
>>>>>> - String month = format((getMonth() + 1), 2);
>>>>>> - String date = format(getDate(), 2);
>>>>>> - String hours = format(getHours(), 2);
>>>>>> - String minutes = format(getMinutes(), 2);
>>>>>> - String seconds = format(getSeconds(), 2);
>>>>>> -
>>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>>>> minutes
>>>>>> - + ':' + seconds + '.' + theNanos;
>>>>>> - }
>>>>>> + StringBuilder sb = new StringBuilder(29);
>>>>>>
>>>>>> - /*
>>>>>> - * Private method to format the time
>>>>>> - */
>>>>>> - private String format(int date, int digits) {
>>>>>> - StringBuilder dateStringBuffer = new
>>>>>> StringBuilder(String.valueOf(date));
>>>>>> - while (dateStringBuffer.length() < digits) {
>>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>>>> + format((getYear() + 1900), 4, sb);
>>>>>> + sb.append('-');
>>>>>> + format((getMonth() + 1), 2, sb);
>>>>>> + sb.append('-');
>>>>>> + format(getDate(), 2, sb);
>>>>>> + sb.append(' ');
>>>>>> + format(getHours(), 2, sb);
>>>>>> + sb.append(':');
>>>>>> + format(getMinutes(), 2, sb);
>>>>>> + sb.append(':');
>>>>>> + format(getSeconds(), 2, sb);
>>>>>> + sb.append('.');
>>>>>> + if (nanos == 0) {
>>>>>> + sb.append('0');
>>>>>> + } else {
>>>>>> + format(nanos, 9, sb);
>>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>>>> + sb.setLength(sb.length() - 1);
>>>>>> + }
>>>>>> }
>>>>>> - return dateStringBuffer.toString();
>>>>>> +
>>>>>> + return sb.toString();
>>>>>> }
>>>>>>
>>>>>> - /*
>>>>>> - * Private method to strip trailing '0' characters from a string.
>>>>>> @param
>>>>>> - * inputString the starting string @return a string with the
>>>>>> trailing zeros
>>>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>>>> - */
>>>>>> - private String stripTrailingZeros(String inputString) {
>>>>>> - String finalString;
>>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>>>
>>>>>> - int i;
>>>>>> - for (i = inputString.length(); i > 0; i--) {
>>>>>> - if (inputString.charAt(i - 1) != '0') {
>>>>>> - break;
>>>>>> - }
>>>>>> - /*
>>>>>> - * If the string has a 0 as its first character, return a
>>>>>> string
>>>>>> - * with a single '0'
>>>>>> - */
>>>>>> - if (i == 1) {
>>>>>> - return "0"; //$NON-NLS-1$
>>>>>> - }
>>>>>> + /*
>>>>>> + * Private method to format the time
>>>>>> + */
>>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>>> + String str = String.valueOf(date);
>>>>>> + if (digits - str.length() > 0) {
>>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>>> }
>>>>>> -
>>>>>> - finalString = inputString.substring(0, i);
>>>>>> - return finalString;
>>>>>> + sb.append(str);
>>>>>> }
>>>>>>
>>>>>> /**
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Best Regards
>>>> Sean, Xiao Xia Qiu
>>>>
>>>> China Software Development Lab, IBM
>>>>
>>>
>>> --
>>> Sent from Gmail for mobile | mobile.google.com
>>>
>>
>>
>>
>> --
>> Best Regards
>> Sean, Xiao Xia Qiu
>>
>> China Software Development Lab, IBM
>>
>
--
Tony Wu
China Software Development Lab, IBM
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nd...@apache.org>.
I looked at the test in further detail and they seem okay. The tests
seem odd though, as they set TimeZone values and mention things like
TimeZone affecting the output. That shouldn't be the case though,
correct? The formats are all in UTC/GMT, correct?
-Nathan
On Wed, Aug 27, 2008 at 8:20 PM, Sean Qiu <se...@gmail.com> wrote:
> As Regis said, there exist tests for these toString method, which,
> IMHO, are covering this modfication.
> Shall we add more tests?
>
> 2008/8/28 Nathan Beyer <nb...@gmail.com>:
>> I was referring to a functional test, not a performance test.
>>
>>
>>
>> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>>> Here is a small test from Regis:
>>>
>>> ----------
>>> int count = 5000;
>>>
>>> for (int i = 0; i < count; ++i) { new Date(new
>>> java.util.Date().getTime()).toString(); }
>>>
>>> Date date = new Date(new java.util.Date().getTime());
>>> long start = System.currentTimeMillis();
>>>
>>> for (int i = 0; i < count; ++i) { date.toString(); }
>>> long end = System.currentTimeMillis();
>>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>>> " + (end - start));
>>>
>>> Time time = new Time(new java.util.Date().getTime());
>>> start = System.currentTimeMillis();
>>> for (int i = 0; i < count; ++i) { time.toString(); }
>>> end = System.currentTimeMillis();
>>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>>> " + (end - start));
>>>
>>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>>> start = System.currentTimeMillis();
>>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>>> end = System.currentTimeMillis();
>>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>>> cost: " + (end - start));
>>> -------------
>>>
>>> the first loop, give time for jvm to warm up
>>>
>>> Below data compare the two implementations:
>>>
>>> before the patch:
>>> Invoke Date.toString() 5000 times, cost: 6757
>>> Invoke Time.toString() 5000 times, cost: 7699
>>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>>
>>> after the patch:
>>> Invoke Date.toString() 5000 times, cost: 84
>>> Invoke Time.toString() 5000 times, cost: 95
>>> Invoke Timestamp.toString() 5000 times, cost: 272
>>>
>>>
>>> We can gain obvious improvement.
>>>
>>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>>> Are there any associated test cases with this change? On a quick
>>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>>> not, we need to start requiring some test cases for "simple
>>>> improvements" like this to continue functional correctness.
>>>>
>>>> -Nathan
>>>>
>>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>>> Author: qiuxx
>>>>> Date: Tue Aug 26 01:59:50 2008
>>>>> New Revision: 689001
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>>> Log:
>>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>>
>>>>> Modified:
>>>>>
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>>
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>>
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>>
>>>>> Modified:
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>> ==============================================================================
>>>>> ---
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>> (original)
>>>>> +++
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>> Tue Aug 26 01:59:50 2008
>>>>> @@ -17,8 +17,6 @@
>>>>>
>>>>> package java.sql;
>>>>>
>>>>> -import java.text.SimpleDateFormat;
>>>>> -
>>>>> /**
>>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>>> * <p>
>>>>> @@ -175,8 +173,28 @@
>>>>> */
>>>>> @Override
>>>>> public String toString() {
>>>>> - SimpleDateFormat dateFormat = new
>>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>>> - return dateFormat.format(this);
>>>>> + StringBuilder sb = new StringBuilder(10);
>>>>> +
>>>>> + format((getYear() + 1900), 4, sb);
>>>>> + sb.append('-');
>>>>> + format((getMonth() + 1), 2, sb);
>>>>> + sb.append('-');
>>>>> + format(getDate(), 2, sb);
>>>>> +
>>>>> + return sb.toString();
>>>>> + }
>>>>> +
>>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>>> +
>>>>> + /*
>>>>> + * Private method to format the time
>>>>> + */
>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>> + String str = String.valueOf(date);
>>>>> + if (digits - str.length() > 0) {
>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>> + }
>>>>> + sb.append(str);
>>>>> }
>>>>>
>>>>> /**
>>>>>
>>>>> Modified:
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>> ==============================================================================
>>>>> ---
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>> (original)
>>>>> +++
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>> Tue Aug 26 01:59:50 2008
>>>>> @@ -17,7 +17,6 @@
>>>>>
>>>>> package java.sql;
>>>>>
>>>>> -import java.text.SimpleDateFormat;
>>>>> import java.util.Date;
>>>>>
>>>>> /**
>>>>> @@ -180,8 +179,28 @@
>>>>> */
>>>>> @Override
>>>>> public String toString() {
>>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>>> //$NON-NLS-1$
>>>>> - return dateFormat.format(this);
>>>>> + StringBuilder sb = new StringBuilder(8);
>>>>> +
>>>>> + format(getHours(), 2, sb);
>>>>> + sb.append(':');
>>>>> + format(getMinutes(), 2, sb);
>>>>> + sb.append(':');
>>>>> + format(getSeconds(), 2, sb);
>>>>> +
>>>>> + return sb.toString();
>>>>> + }
>>>>> +
>>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>>> +
>>>>> + /*
>>>>> + * Private method to format the time
>>>>> + */
>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>> + String str = String.valueOf(date);
>>>>> + if (digits - str.length() > 0) {
>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>> + }
>>>>> + sb.append(str);
>>>>> }
>>>>>
>>>>> /**
>>>>>
>>>>> Modified:
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>>> ==============================================================================
>>>>> ---
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>> (original)
>>>>> +++
>>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>> Tue Aug 26 01:59:50 2008
>>>>> @@ -309,62 +309,43 @@
>>>>> @SuppressWarnings("deprecation")
>>>>> @Override
>>>>> public String toString() {
>>>>> - /*
>>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>>> simple
>>>>> - * string of 9 integers, with leading Zeros
>>>>> - */
>>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>>> //$NON-NLS-1$
>>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>>> - String theNanos = decimalFormat.format(nanos);
>>>>> - theNanos = stripTrailingZeros(theNanos);
>>>>> -
>>>>> - String year = format((getYear() + 1900), 4);
>>>>> - String month = format((getMonth() + 1), 2);
>>>>> - String date = format(getDate(), 2);
>>>>> - String hours = format(getHours(), 2);
>>>>> - String minutes = format(getMinutes(), 2);
>>>>> - String seconds = format(getSeconds(), 2);
>>>>> -
>>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>>> minutes
>>>>> - + ':' + seconds + '.' + theNanos;
>>>>> - }
>>>>> + StringBuilder sb = new StringBuilder(29);
>>>>>
>>>>> - /*
>>>>> - * Private method to format the time
>>>>> - */
>>>>> - private String format(int date, int digits) {
>>>>> - StringBuilder dateStringBuffer = new
>>>>> StringBuilder(String.valueOf(date));
>>>>> - while (dateStringBuffer.length() < digits) {
>>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>>> + format((getYear() + 1900), 4, sb);
>>>>> + sb.append('-');
>>>>> + format((getMonth() + 1), 2, sb);
>>>>> + sb.append('-');
>>>>> + format(getDate(), 2, sb);
>>>>> + sb.append(' ');
>>>>> + format(getHours(), 2, sb);
>>>>> + sb.append(':');
>>>>> + format(getMinutes(), 2, sb);
>>>>> + sb.append(':');
>>>>> + format(getSeconds(), 2, sb);
>>>>> + sb.append('.');
>>>>> + if (nanos == 0) {
>>>>> + sb.append('0');
>>>>> + } else {
>>>>> + format(nanos, 9, sb);
>>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>>> + sb.setLength(sb.length() - 1);
>>>>> + }
>>>>> }
>>>>> - return dateStringBuffer.toString();
>>>>> +
>>>>> + return sb.toString();
>>>>> }
>>>>>
>>>>> - /*
>>>>> - * Private method to strip trailing '0' characters from a string.
>>>>> @param
>>>>> - * inputString the starting string @return a string with the
>>>>> trailing zeros
>>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>>> - */
>>>>> - private String stripTrailingZeros(String inputString) {
>>>>> - String finalString;
>>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>>
>>>>> - int i;
>>>>> - for (i = inputString.length(); i > 0; i--) {
>>>>> - if (inputString.charAt(i - 1) != '0') {
>>>>> - break;
>>>>> - }
>>>>> - /*
>>>>> - * If the string has a 0 as its first character, return a
>>>>> string
>>>>> - * with a single '0'
>>>>> - */
>>>>> - if (i == 1) {
>>>>> - return "0"; //$NON-NLS-1$
>>>>> - }
>>>>> + /*
>>>>> + * Private method to format the time
>>>>> + */
>>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>>> + String str = String.valueOf(date);
>>>>> + if (digits - str.length() > 0) {
>>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>>> }
>>>>> -
>>>>> - finalString = inputString.substring(0, i);
>>>>> - return finalString;
>>>>> + sb.append(str);
>>>>> }
>>>>>
>>>>> /**
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Best Regards
>>> Sean, Xiao Xia Qiu
>>>
>>> China Software Development Lab, IBM
>>>
>>
>> --
>> Sent from Gmail for mobile | mobile.google.com
>>
>
>
>
> --
> Best Regards
> Sean, Xiao Xia Qiu
>
> China Software Development Lab, IBM
>
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Sean Qiu <se...@gmail.com>.
As Regis said, there exist tests for these toString method, which,
IMHO, are covering this modfication.
Shall we add more tests?
2008/8/28 Nathan Beyer <nb...@gmail.com>:
> I was referring to a functional test, not a performance test.
>
>
>
> On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
>> Here is a small test from Regis:
>>
>> ----------
>> int count = 5000;
>>
>> for (int i = 0; i < count; ++i) { new Date(new
>> java.util.Date().getTime()).toString(); }
>>
>> Date date = new Date(new java.util.Date().getTime());
>> long start = System.currentTimeMillis();
>>
>> for (int i = 0; i < count; ++i) { date.toString(); }
>> long end = System.currentTimeMillis();
>> System.out.println("Invoke Date.toString() " + count + " times, cost:
>> " + (end - start));
>>
>> Time time = new Time(new java.util.Date().getTime());
>> start = System.currentTimeMillis();
>> for (int i = 0; i < count; ++i) { time.toString(); }
>> end = System.currentTimeMillis();
>> System.out.println("Invoke Time.toString() " + count + " times, cost:
>> " + (end - start));
>>
>> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
>> start = System.currentTimeMillis();
>> for (int i = 0; i < count; ++i) { timestamp.toString(); }
>> end = System.currentTimeMillis();
>> System.out.println("Invoke Timestamp.toString() " + count + " times,
>> cost: " + (end - start));
>> -------------
>>
>> the first loop, give time for jvm to warm up
>>
>> Below data compare the two implementations:
>>
>> before the patch:
>> Invoke Date.toString() 5000 times, cost: 6757
>> Invoke Time.toString() 5000 times, cost: 7699
>> Invoke Timestamp.toString() 5000 times, cost: 2527
>>
>> after the patch:
>> Invoke Date.toString() 5000 times, cost: 84
>> Invoke Time.toString() 5000 times, cost: 95
>> Invoke Timestamp.toString() 5000 times, cost: 272
>>
>>
>> We can gain obvious improvement.
>>
>> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>>> Are there any associated test cases with this change? On a quick
>>> cursory look, I didn't see any existing tests. Did I miss them? If
>>> not, we need to start requiring some test cases for "simple
>>> improvements" like this to continue functional correctness.
>>>
>>> -Nathan
>>>
>>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>>> Author: qiuxx
>>>> Date: Tue Aug 26 01:59:50 2008
>>>> New Revision: 689001
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>>> Log:
>>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>>
>>>> Modified:
>>>>
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>>
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>>
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>>
>>>> Modified:
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>>> ==============================================================================
>>>> ---
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>> (original)
>>>> +++
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>> Tue Aug 26 01:59:50 2008
>>>> @@ -17,8 +17,6 @@
>>>>
>>>> package java.sql;
>>>>
>>>> -import java.text.SimpleDateFormat;
>>>> -
>>>> /**
>>>> * A Date class which can consume and produce dates in SQL Date format.
>>>> * <p>
>>>> @@ -175,8 +173,28 @@
>>>> */
>>>> @Override
>>>> public String toString() {
>>>> - SimpleDateFormat dateFormat = new
>>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>>> - return dateFormat.format(this);
>>>> + StringBuilder sb = new StringBuilder(10);
>>>> +
>>>> + format((getYear() + 1900), 4, sb);
>>>> + sb.append('-');
>>>> + format((getMonth() + 1), 2, sb);
>>>> + sb.append('-');
>>>> + format(getDate(), 2, sb);
>>>> +
>>>> + return sb.toString();
>>>> + }
>>>> +
>>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>>> +
>>>> + /*
>>>> + * Private method to format the time
>>>> + */
>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>> + String str = String.valueOf(date);
>>>> + if (digits - str.length() > 0) {
>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>> + }
>>>> + sb.append(str);
>>>> }
>>>>
>>>> /**
>>>>
>>>> Modified:
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>>> ==============================================================================
>>>> ---
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>> (original)
>>>> +++
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>> Tue Aug 26 01:59:50 2008
>>>> @@ -17,7 +17,6 @@
>>>>
>>>> package java.sql;
>>>>
>>>> -import java.text.SimpleDateFormat;
>>>> import java.util.Date;
>>>>
>>>> /**
>>>> @@ -180,8 +179,28 @@
>>>> */
>>>> @Override
>>>> public String toString() {
>>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>>> //$NON-NLS-1$
>>>> - return dateFormat.format(this);
>>>> + StringBuilder sb = new StringBuilder(8);
>>>> +
>>>> + format(getHours(), 2, sb);
>>>> + sb.append(':');
>>>> + format(getMinutes(), 2, sb);
>>>> + sb.append(':');
>>>> + format(getSeconds(), 2, sb);
>>>> +
>>>> + return sb.toString();
>>>> + }
>>>> +
>>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>>> +
>>>> + /*
>>>> + * Private method to format the time
>>>> + */
>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>> + String str = String.valueOf(date);
>>>> + if (digits - str.length() > 0) {
>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>> + }
>>>> + sb.append(str);
>>>> }
>>>>
>>>> /**
>>>>
>>>> Modified:
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>>> ==============================================================================
>>>> ---
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>> (original)
>>>> +++
>>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>> Tue Aug 26 01:59:50 2008
>>>> @@ -309,62 +309,43 @@
>>>> @SuppressWarnings("deprecation")
>>>> @Override
>>>> public String toString() {
>>>> - /*
>>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>>> simple
>>>> - * string of 9 integers, with leading Zeros
>>>> - */
>>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>>> //$NON-NLS-1$
>>>> - decimalFormat.setMinimumIntegerDigits(9);
>>>> - decimalFormat.setMaximumIntegerDigits(9);
>>>> - String theNanos = decimalFormat.format(nanos);
>>>> - theNanos = stripTrailingZeros(theNanos);
>>>> -
>>>> - String year = format((getYear() + 1900), 4);
>>>> - String month = format((getMonth() + 1), 2);
>>>> - String date = format(getDate(), 2);
>>>> - String hours = format(getHours(), 2);
>>>> - String minutes = format(getMinutes(), 2);
>>>> - String seconds = format(getSeconds(), 2);
>>>> -
>>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>>> minutes
>>>> - + ':' + seconds + '.' + theNanos;
>>>> - }
>>>> + StringBuilder sb = new StringBuilder(29);
>>>>
>>>> - /*
>>>> - * Private method to format the time
>>>> - */
>>>> - private String format(int date, int digits) {
>>>> - StringBuilder dateStringBuffer = new
>>>> StringBuilder(String.valueOf(date));
>>>> - while (dateStringBuffer.length() < digits) {
>>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>>> + format((getYear() + 1900), 4, sb);
>>>> + sb.append('-');
>>>> + format((getMonth() + 1), 2, sb);
>>>> + sb.append('-');
>>>> + format(getDate(), 2, sb);
>>>> + sb.append(' ');
>>>> + format(getHours(), 2, sb);
>>>> + sb.append(':');
>>>> + format(getMinutes(), 2, sb);
>>>> + sb.append(':');
>>>> + format(getSeconds(), 2, sb);
>>>> + sb.append('.');
>>>> + if (nanos == 0) {
>>>> + sb.append('0');
>>>> + } else {
>>>> + format(nanos, 9, sb);
>>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>>> + sb.setLength(sb.length() - 1);
>>>> + }
>>>> }
>>>> - return dateStringBuffer.toString();
>>>> +
>>>> + return sb.toString();
>>>> }
>>>>
>>>> - /*
>>>> - * Private method to strip trailing '0' characters from a string.
>>>> @param
>>>> - * inputString the starting string @return a string with the
>>>> trailing zeros
>>>> - * stripped - will leave a single 0 at the beginning of the string
>>>> - */
>>>> - private String stripTrailingZeros(String inputString) {
>>>> - String finalString;
>>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>>
>>>> - int i;
>>>> - for (i = inputString.length(); i > 0; i--) {
>>>> - if (inputString.charAt(i - 1) != '0') {
>>>> - break;
>>>> - }
>>>> - /*
>>>> - * If the string has a 0 as its first character, return a
>>>> string
>>>> - * with a single '0'
>>>> - */
>>>> - if (i == 1) {
>>>> - return "0"; //$NON-NLS-1$
>>>> - }
>>>> + /*
>>>> + * Private method to format the time
>>>> + */
>>>> + private void format(int date, int digits, StringBuilder sb) {
>>>> + String str = String.valueOf(date);
>>>> + if (digits - str.length() > 0) {
>>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>>> }
>>>> -
>>>> - finalString = inputString.substring(0, i);
>>>> - return finalString;
>>>> + sb.append(str);
>>>> }
>>>>
>>>> /**
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>> --
>> Best Regards
>> Sean, Xiao Xia Qiu
>>
>> China Software Development Lab, IBM
>>
>
> --
> Sent from Gmail for mobile | mobile.google.com
>
--
Best Regards
Sean, Xiao Xia Qiu
China Software Development Lab, IBM
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nb...@gmail.com>.
I was referring to a functional test, not a performance test.
On 8/27/08, Sean Qiu <se...@gmail.com> wrote:
> Here is a small test from Regis:
>
> ----------
> int count = 5000;
>
> for (int i = 0; i < count; ++i) { new Date(new
> java.util.Date().getTime()).toString(); }
>
> Date date = new Date(new java.util.Date().getTime());
> long start = System.currentTimeMillis();
>
> for (int i = 0; i < count; ++i) { date.toString(); }
> long end = System.currentTimeMillis();
> System.out.println("Invoke Date.toString() " + count + " times, cost:
> " + (end - start));
>
> Time time = new Time(new java.util.Date().getTime());
> start = System.currentTimeMillis();
> for (int i = 0; i < count; ++i) { time.toString(); }
> end = System.currentTimeMillis();
> System.out.println("Invoke Time.toString() " + count + " times, cost:
> " + (end - start));
>
> Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
> start = System.currentTimeMillis();
> for (int i = 0; i < count; ++i) { timestamp.toString(); }
> end = System.currentTimeMillis();
> System.out.println("Invoke Timestamp.toString() " + count + " times,
> cost: " + (end - start));
> -------------
>
> the first loop, give time for jvm to warm up
>
> Below data compare the two implementations:
>
> before the patch:
> Invoke Date.toString() 5000 times, cost: 6757
> Invoke Time.toString() 5000 times, cost: 7699
> Invoke Timestamp.toString() 5000 times, cost: 2527
>
> after the patch:
> Invoke Date.toString() 5000 times, cost: 84
> Invoke Time.toString() 5000 times, cost: 95
> Invoke Timestamp.toString() 5000 times, cost: 272
>
>
> We can gain obvious improvement.
>
> 2008/8/28 Nathan Beyer <nd...@apache.org>:
>> Are there any associated test cases with this change? On a quick
>> cursory look, I didn't see any existing tests. Did I miss them? If
>> not, we need to start requiring some test cases for "simple
>> improvements" like this to continue functional correctness.
>>
>> -Nathan
>>
>> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>>> Author: qiuxx
>>> Date: Tue Aug 26 01:59:50 2008
>>> New Revision: 689001
>>>
>>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>>> Log:
>>> Apply for HARMONY-5958,([classlib][sql][performance] - improve
>>> performance of java.sql.Date/Time/Timestamp.toString())
>>>
>>> Modified:
>>>
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>>
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>>
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>>
>>> Modified:
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>> URL:
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>>> ==============================================================================
>>> ---
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>> (original)
>>> +++
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>>> Tue Aug 26 01:59:50 2008
>>> @@ -17,8 +17,6 @@
>>>
>>> package java.sql;
>>>
>>> -import java.text.SimpleDateFormat;
>>> -
>>> /**
>>> * A Date class which can consume and produce dates in SQL Date format.
>>> * <p>
>>> @@ -175,8 +173,28 @@
>>> */
>>> @Override
>>> public String toString() {
>>> - SimpleDateFormat dateFormat = new
>>> SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>>> - return dateFormat.format(this);
>>> + StringBuilder sb = new StringBuilder(10);
>>> +
>>> + format((getYear() + 1900), 4, sb);
>>> + sb.append('-');
>>> + format((getMonth() + 1), 2, sb);
>>> + sb.append('-');
>>> + format(getDate(), 2, sb);
>>> +
>>> + return sb.toString();
>>> + }
>>> +
>>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>>> +
>>> + /*
>>> + * Private method to format the time
>>> + */
>>> + private void format(int date, int digits, StringBuilder sb) {
>>> + String str = String.valueOf(date);
>>> + if (digits - str.length() > 0) {
>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>> + }
>>> + sb.append(str);
>>> }
>>>
>>> /**
>>>
>>> Modified:
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>> URL:
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>>> ==============================================================================
>>> ---
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>> (original)
>>> +++
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>>> Tue Aug 26 01:59:50 2008
>>> @@ -17,7 +17,6 @@
>>>
>>> package java.sql;
>>>
>>> -import java.text.SimpleDateFormat;
>>> import java.util.Date;
>>>
>>> /**
>>> @@ -180,8 +179,28 @@
>>> */
>>> @Override
>>> public String toString() {
>>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
>>> //$NON-NLS-1$
>>> - return dateFormat.format(this);
>>> + StringBuilder sb = new StringBuilder(8);
>>> +
>>> + format(getHours(), 2, sb);
>>> + sb.append(':');
>>> + format(getMinutes(), 2, sb);
>>> + sb.append(':');
>>> + format(getSeconds(), 2, sb);
>>> +
>>> + return sb.toString();
>>> + }
>>> +
>>> + private static final String PADDING = "00"; //$NON-NLS-1$
>>> +
>>> + /*
>>> + * Private method to format the time
>>> + */
>>> + private void format(int date, int digits, StringBuilder sb) {
>>> + String str = String.valueOf(date);
>>> + if (digits - str.length() > 0) {
>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>> + }
>>> + sb.append(str);
>>> }
>>>
>>> /**
>>>
>>> Modified:
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>> URL:
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>>> ==============================================================================
>>> ---
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>> (original)
>>> +++
>>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>> Tue Aug 26 01:59:50 2008
>>> @@ -309,62 +309,43 @@
>>> @SuppressWarnings("deprecation")
>>> @Override
>>> public String toString() {
>>> - /*
>>> - * Use a DecimalFormat to lay out the nanosecond value as a
>>> simple
>>> - * string of 9 integers, with leading Zeros
>>> - */
>>> - DecimalFormat decimalFormat = new DecimalFormat("0");
>>> //$NON-NLS-1$
>>> - decimalFormat.setMinimumIntegerDigits(9);
>>> - decimalFormat.setMaximumIntegerDigits(9);
>>> - String theNanos = decimalFormat.format(nanos);
>>> - theNanos = stripTrailingZeros(theNanos);
>>> -
>>> - String year = format((getYear() + 1900), 4);
>>> - String month = format((getMonth() + 1), 2);
>>> - String date = format(getDate(), 2);
>>> - String hours = format(getHours(), 2);
>>> - String minutes = format(getMinutes(), 2);
>>> - String seconds = format(getSeconds(), 2);
>>> -
>>> - return year + '-' + month + '-' + date + ' ' + hours + ':' +
>>> minutes
>>> - + ':' + seconds + '.' + theNanos;
>>> - }
>>> + StringBuilder sb = new StringBuilder(29);
>>>
>>> - /*
>>> - * Private method to format the time
>>> - */
>>> - private String format(int date, int digits) {
>>> - StringBuilder dateStringBuffer = new
>>> StringBuilder(String.valueOf(date));
>>> - while (dateStringBuffer.length() < digits) {
>>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>>> + format((getYear() + 1900), 4, sb);
>>> + sb.append('-');
>>> + format((getMonth() + 1), 2, sb);
>>> + sb.append('-');
>>> + format(getDate(), 2, sb);
>>> + sb.append(' ');
>>> + format(getHours(), 2, sb);
>>> + sb.append(':');
>>> + format(getMinutes(), 2, sb);
>>> + sb.append(':');
>>> + format(getSeconds(), 2, sb);
>>> + sb.append('.');
>>> + if (nanos == 0) {
>>> + sb.append('0');
>>> + } else {
>>> + format(nanos, 9, sb);
>>> + while (sb.charAt(sb.length() - 1) == '0') {
>>> + sb.setLength(sb.length() - 1);
>>> + }
>>> }
>>> - return dateStringBuffer.toString();
>>> +
>>> + return sb.toString();
>>> }
>>>
>>> - /*
>>> - * Private method to strip trailing '0' characters from a string.
>>> @param
>>> - * inputString the starting string @return a string with the
>>> trailing zeros
>>> - * stripped - will leave a single 0 at the beginning of the string
>>> - */
>>> - private String stripTrailingZeros(String inputString) {
>>> - String finalString;
>>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>>
>>> - int i;
>>> - for (i = inputString.length(); i > 0; i--) {
>>> - if (inputString.charAt(i - 1) != '0') {
>>> - break;
>>> - }
>>> - /*
>>> - * If the string has a 0 as its first character, return a
>>> string
>>> - * with a single '0'
>>> - */
>>> - if (i == 1) {
>>> - return "0"; //$NON-NLS-1$
>>> - }
>>> + /*
>>> + * Private method to format the time
>>> + */
>>> + private void format(int date, int digits, StringBuilder sb) {
>>> + String str = String.valueOf(date);
>>> + if (digits - str.length() > 0) {
>>> + sb.append(PADDING.substring(0, digits - str.length()));
>>> }
>>> -
>>> - finalString = inputString.substring(0, i);
>>> - return finalString;
>>> + sb.append(str);
>>> }
>>>
>>> /**
>>>
>>>
>>>
>>
>
>
>
> --
> Best Regards
> Sean, Xiao Xia Qiu
>
> China Software Development Lab, IBM
>
--
Sent from Gmail for mobile | mobile.google.com
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Sean Qiu <se...@gmail.com>.
Here is a small test from Regis:
----------
int count = 5000;
for (int i = 0; i < count; ++i) { new Date(new
java.util.Date().getTime()).toString(); }
Date date = new Date(new java.util.Date().getTime());
long start = System.currentTimeMillis();
for (int i = 0; i < count; ++i) { date.toString(); }
long end = System.currentTimeMillis();
System.out.println("Invoke Date.toString() " + count + " times, cost:
" + (end - start));
Time time = new Time(new java.util.Date().getTime());
start = System.currentTimeMillis();
for (int i = 0; i < count; ++i) { time.toString(); }
end = System.currentTimeMillis();
System.out.println("Invoke Time.toString() " + count + " times, cost:
" + (end - start));
Timestamp timestamp = new Timestamp(new java.util.Date().getTime());
start = System.currentTimeMillis();
for (int i = 0; i < count; ++i) { timestamp.toString(); }
end = System.currentTimeMillis();
System.out.println("Invoke Timestamp.toString() " + count + " times,
cost: " + (end - start));
-------------
the first loop, give time for jvm to warm up
Below data compare the two implementations:
before the patch:
Invoke Date.toString() 5000 times, cost: 6757
Invoke Time.toString() 5000 times, cost: 7699
Invoke Timestamp.toString() 5000 times, cost: 2527
after the patch:
Invoke Date.toString() 5000 times, cost: 84
Invoke Time.toString() 5000 times, cost: 95
Invoke Timestamp.toString() 5000 times, cost: 272
We can gain obvious improvement.
2008/8/28 Nathan Beyer <nd...@apache.org>:
> Are there any associated test cases with this change? On a quick
> cursory look, I didn't see any existing tests. Did I miss them? If
> not, we need to start requiring some test cases for "simple
> improvements" like this to continue functional correctness.
>
> -Nathan
>
> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>> Author: qiuxx
>> Date: Tue Aug 26 01:59:50 2008
>> New Revision: 689001
>>
>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>> Log:
>> Apply for HARMONY-5958,([classlib][sql][performance] - improve performance of java.sql.Date/Time/Timestamp.toString())
>>
>> Modified:
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java Tue Aug 26 01:59:50 2008
>> @@ -17,8 +17,6 @@
>>
>> package java.sql;
>>
>> -import java.text.SimpleDateFormat;
>> -
>> /**
>> * A Date class which can consume and produce dates in SQL Date format.
>> * <p>
>> @@ -175,8 +173,28 @@
>> */
>> @Override
>> public String toString() {
>> - SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>> - return dateFormat.format(this);
>> + StringBuilder sb = new StringBuilder(10);
>> +
>> + format((getYear() + 1900), 4, sb);
>> + sb.append('-');
>> + format((getMonth() + 1), 2, sb);
>> + sb.append('-');
>> + format(getDate(), 2, sb);
>> +
>> + return sb.toString();
>> + }
>> +
>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>> +
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> + }
>> + sb.append(str);
>> }
>>
>> /**
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java Tue Aug 26 01:59:50 2008
>> @@ -17,7 +17,6 @@
>>
>> package java.sql;
>>
>> -import java.text.SimpleDateFormat;
>> import java.util.Date;
>>
>> /**
>> @@ -180,8 +179,28 @@
>> */
>> @Override
>> public String toString() {
>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss"); //$NON-NLS-1$
>> - return dateFormat.format(this);
>> + StringBuilder sb = new StringBuilder(8);
>> +
>> + format(getHours(), 2, sb);
>> + sb.append(':');
>> + format(getMinutes(), 2, sb);
>> + sb.append(':');
>> + format(getSeconds(), 2, sb);
>> +
>> + return sb.toString();
>> + }
>> +
>> + private static final String PADDING = "00"; //$NON-NLS-1$
>> +
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> + }
>> + sb.append(str);
>> }
>>
>> /**
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java Tue Aug 26 01:59:50 2008
>> @@ -309,62 +309,43 @@
>> @SuppressWarnings("deprecation")
>> @Override
>> public String toString() {
>> - /*
>> - * Use a DecimalFormat to lay out the nanosecond value as a simple
>> - * string of 9 integers, with leading Zeros
>> - */
>> - DecimalFormat decimalFormat = new DecimalFormat("0"); //$NON-NLS-1$
>> - decimalFormat.setMinimumIntegerDigits(9);
>> - decimalFormat.setMaximumIntegerDigits(9);
>> - String theNanos = decimalFormat.format(nanos);
>> - theNanos = stripTrailingZeros(theNanos);
>> -
>> - String year = format((getYear() + 1900), 4);
>> - String month = format((getMonth() + 1), 2);
>> - String date = format(getDate(), 2);
>> - String hours = format(getHours(), 2);
>> - String minutes = format(getMinutes(), 2);
>> - String seconds = format(getSeconds(), 2);
>> -
>> - return year + '-' + month + '-' + date + ' ' + hours + ':' + minutes
>> - + ':' + seconds + '.' + theNanos;
>> - }
>> + StringBuilder sb = new StringBuilder(29);
>>
>> - /*
>> - * Private method to format the time
>> - */
>> - private String format(int date, int digits) {
>> - StringBuilder dateStringBuffer = new StringBuilder(String.valueOf(date));
>> - while (dateStringBuffer.length() < digits) {
>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>> + format((getYear() + 1900), 4, sb);
>> + sb.append('-');
>> + format((getMonth() + 1), 2, sb);
>> + sb.append('-');
>> + format(getDate(), 2, sb);
>> + sb.append(' ');
>> + format(getHours(), 2, sb);
>> + sb.append(':');
>> + format(getMinutes(), 2, sb);
>> + sb.append(':');
>> + format(getSeconds(), 2, sb);
>> + sb.append('.');
>> + if (nanos == 0) {
>> + sb.append('0');
>> + } else {
>> + format(nanos, 9, sb);
>> + while (sb.charAt(sb.length() - 1) == '0') {
>> + sb.setLength(sb.length() - 1);
>> + }
>> }
>> - return dateStringBuffer.toString();
>> +
>> + return sb.toString();
>> }
>>
>> - /*
>> - * Private method to strip trailing '0' characters from a string. @param
>> - * inputString the starting string @return a string with the trailing zeros
>> - * stripped - will leave a single 0 at the beginning of the string
>> - */
>> - private String stripTrailingZeros(String inputString) {
>> - String finalString;
>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>
>> - int i;
>> - for (i = inputString.length(); i > 0; i--) {
>> - if (inputString.charAt(i - 1) != '0') {
>> - break;
>> - }
>> - /*
>> - * If the string has a 0 as its first character, return a string
>> - * with a single '0'
>> - */
>> - if (i == 1) {
>> - return "0"; //$NON-NLS-1$
>> - }
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> }
>> -
>> - finalString = inputString.substring(0, i);
>> - return finalString;
>> + sb.append(str);
>> }
>>
>> /**
>>
>>
>>
>
--
Best Regards
Sean, Xiao Xia Qiu
China Software Development Lab, IBM
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql:
Date.java Time.java Timestamp.java
Posted by Regis <xu...@gmail.com>.
Hi Nathan,
Thanks for your review. Actually there are tests for toString() methods in
org.apache.harmony.sql.tests.java.sql. DateTest/TimeTest/Timestamp.
I think these tests are enough, since the functions are not so complex, and
the new implementation passed all tests, so I didn't add new tests for this.
If you found new bugs introduced by the issue, I would like to fix them.
Best Regards,
Regis.
Nathan Beyer wrote:
> Are there any associated test cases with this change? On a quick
> cursory look, I didn't see any existing tests. Did I miss them? If
> not, we need to start requiring some test cases for "simple
> improvements" like this to continue functional correctness.
>
> -Nathan
>
> On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
>> Author: qiuxx
>> Date: Tue Aug 26 01:59:50 2008
>> New Revision: 689001
>>
>> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
>> Log:
>> Apply for HARMONY-5958,([classlib][sql][performance] - improve performance of java.sql.Date/Time/Timestamp.toString())
>>
>> Modified:
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java Tue Aug 26 01:59:50 2008
>> @@ -17,8 +17,6 @@
>>
>> package java.sql;
>>
>> -import java.text.SimpleDateFormat;
>> -
>> /**
>> * A Date class which can consume and produce dates in SQL Date format.
>> * <p>
>> @@ -175,8 +173,28 @@
>> */
>> @Override
>> public String toString() {
>> - SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
>> - return dateFormat.format(this);
>> + StringBuilder sb = new StringBuilder(10);
>> +
>> + format((getYear() + 1900), 4, sb);
>> + sb.append('-');
>> + format((getMonth() + 1), 2, sb);
>> + sb.append('-');
>> + format(getDate(), 2, sb);
>> +
>> + return sb.toString();
>> + }
>> +
>> + private static final String PADDING = "0000"; //$NON-NLS-1$
>> +
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> + }
>> + sb.append(str);
>> }
>>
>> /**
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java Tue Aug 26 01:59:50 2008
>> @@ -17,7 +17,6 @@
>>
>> package java.sql;
>>
>> -import java.text.SimpleDateFormat;
>> import java.util.Date;
>>
>> /**
>> @@ -180,8 +179,28 @@
>> */
>> @Override
>> public String toString() {
>> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss"); //$NON-NLS-1$
>> - return dateFormat.format(this);
>> + StringBuilder sb = new StringBuilder(8);
>> +
>> + format(getHours(), 2, sb);
>> + sb.append(':');
>> + format(getMinutes(), 2, sb);
>> + sb.append(':');
>> + format(getSeconds(), 2, sb);
>> +
>> + return sb.toString();
>> + }
>> +
>> + private static final String PADDING = "00"; //$NON-NLS-1$
>> +
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> + }
>> + sb.append(str);
>> }
>>
>> /**
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java Tue Aug 26 01:59:50 2008
>> @@ -309,62 +309,43 @@
>> @SuppressWarnings("deprecation")
>> @Override
>> public String toString() {
>> - /*
>> - * Use a DecimalFormat to lay out the nanosecond value as a simple
>> - * string of 9 integers, with leading Zeros
>> - */
>> - DecimalFormat decimalFormat = new DecimalFormat("0"); //$NON-NLS-1$
>> - decimalFormat.setMinimumIntegerDigits(9);
>> - decimalFormat.setMaximumIntegerDigits(9);
>> - String theNanos = decimalFormat.format(nanos);
>> - theNanos = stripTrailingZeros(theNanos);
>> -
>> - String year = format((getYear() + 1900), 4);
>> - String month = format((getMonth() + 1), 2);
>> - String date = format(getDate(), 2);
>> - String hours = format(getHours(), 2);
>> - String minutes = format(getMinutes(), 2);
>> - String seconds = format(getSeconds(), 2);
>> -
>> - return year + '-' + month + '-' + date + ' ' + hours + ':' + minutes
>> - + ':' + seconds + '.' + theNanos;
>> - }
>> + StringBuilder sb = new StringBuilder(29);
>>
>> - /*
>> - * Private method to format the time
>> - */
>> - private String format(int date, int digits) {
>> - StringBuilder dateStringBuffer = new StringBuilder(String.valueOf(date));
>> - while (dateStringBuffer.length() < digits) {
>> - dateStringBuffer = dateStringBuffer.insert(0, '0');
>> + format((getYear() + 1900), 4, sb);
>> + sb.append('-');
>> + format((getMonth() + 1), 2, sb);
>> + sb.append('-');
>> + format(getDate(), 2, sb);
>> + sb.append(' ');
>> + format(getHours(), 2, sb);
>> + sb.append(':');
>> + format(getMinutes(), 2, sb);
>> + sb.append(':');
>> + format(getSeconds(), 2, sb);
>> + sb.append('.');
>> + if (nanos == 0) {
>> + sb.append('0');
>> + } else {
>> + format(nanos, 9, sb);
>> + while (sb.charAt(sb.length() - 1) == '0') {
>> + sb.setLength(sb.length() - 1);
>> + }
>> }
>> - return dateStringBuffer.toString();
>> +
>> + return sb.toString();
>> }
>>
>> - /*
>> - * Private method to strip trailing '0' characters from a string. @param
>> - * inputString the starting string @return a string with the trailing zeros
>> - * stripped - will leave a single 0 at the beginning of the string
>> - */
>> - private String stripTrailingZeros(String inputString) {
>> - String finalString;
>> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>>
>> - int i;
>> - for (i = inputString.length(); i > 0; i--) {
>> - if (inputString.charAt(i - 1) != '0') {
>> - break;
>> - }
>> - /*
>> - * If the string has a 0 as its first character, return a string
>> - * with a single '0'
>> - */
>> - if (i == 1) {
>> - return "0"; //$NON-NLS-1$
>> - }
>> + /*
>> + * Private method to format the time
>> + */
>> + private void format(int date, int digits, StringBuilder sb) {
>> + String str = String.valueOf(date);
>> + if (digits - str.length() > 0) {
>> + sb.append(PADDING.substring(0, digits - str.length()));
>> }
>> -
>> - finalString = inputString.substring(0, i);
>> - return finalString;
>> + sb.append(str);
>> }
>>
>> /**
>>
>>
>>
>
Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Posted by Nathan Beyer <nd...@apache.org>.
Are there any associated test cases with this change? On a quick
cursory look, I didn't see any existing tests. Did I miss them? If
not, we need to start requiring some test cases for "simple
improvements" like this to continue functional correctness.
-Nathan
On Tue, Aug 26, 2008 at 3:59 AM, <qi...@apache.org> wrote:
> Author: qiuxx
> Date: Tue Aug 26 01:59:50 2008
> New Revision: 689001
>
> URL: http://svn.apache.org/viewvc?rev=689001&view=rev
> Log:
> Apply for HARMONY-5958,([classlib][sql][performance] - improve performance of java.sql.Date/Time/Timestamp.toString())
>
> Modified:
> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
>
> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java?rev=689001&r1=689000&r2=689001&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Date.java Tue Aug 26 01:59:50 2008
> @@ -17,8 +17,6 @@
>
> package java.sql;
>
> -import java.text.SimpleDateFormat;
> -
> /**
> * A Date class which can consume and produce dates in SQL Date format.
> * <p>
> @@ -175,8 +173,28 @@
> */
> @Override
> public String toString() {
> - SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd"); //$NON-NLS-1$
> - return dateFormat.format(this);
> + StringBuilder sb = new StringBuilder(10);
> +
> + format((getYear() + 1900), 4, sb);
> + sb.append('-');
> + format((getMonth() + 1), 2, sb);
> + sb.append('-');
> + format(getDate(), 2, sb);
> +
> + return sb.toString();
> + }
> +
> + private static final String PADDING = "0000"; //$NON-NLS-1$
> +
> + /*
> + * Private method to format the time
> + */
> + private void format(int date, int digits, StringBuilder sb) {
> + String str = String.valueOf(date);
> + if (digits - str.length() > 0) {
> + sb.append(PADDING.substring(0, digits - str.length()));
> + }
> + sb.append(str);
> }
>
> /**
>
> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java?rev=689001&r1=689000&r2=689001&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Time.java Tue Aug 26 01:59:50 2008
> @@ -17,7 +17,6 @@
>
> package java.sql;
>
> -import java.text.SimpleDateFormat;
> import java.util.Date;
>
> /**
> @@ -180,8 +179,28 @@
> */
> @Override
> public String toString() {
> - SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss"); //$NON-NLS-1$
> - return dateFormat.format(this);
> + StringBuilder sb = new StringBuilder(8);
> +
> + format(getHours(), 2, sb);
> + sb.append(':');
> + format(getMinutes(), 2, sb);
> + sb.append(':');
> + format(getSeconds(), 2, sb);
> +
> + return sb.toString();
> + }
> +
> + private static final String PADDING = "00"; //$NON-NLS-1$
> +
> + /*
> + * Private method to format the time
> + */
> + private void format(int date, int digits, StringBuilder sb) {
> + String str = String.valueOf(date);
> + if (digits - str.length() > 0) {
> + sb.append(PADDING.substring(0, digits - str.length()));
> + }
> + sb.append(str);
> }
>
> /**
>
> Modified: harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java?rev=689001&r1=689000&r2=689001&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql/Timestamp.java Tue Aug 26 01:59:50 2008
> @@ -309,62 +309,43 @@
> @SuppressWarnings("deprecation")
> @Override
> public String toString() {
> - /*
> - * Use a DecimalFormat to lay out the nanosecond value as a simple
> - * string of 9 integers, with leading Zeros
> - */
> - DecimalFormat decimalFormat = new DecimalFormat("0"); //$NON-NLS-1$
> - decimalFormat.setMinimumIntegerDigits(9);
> - decimalFormat.setMaximumIntegerDigits(9);
> - String theNanos = decimalFormat.format(nanos);
> - theNanos = stripTrailingZeros(theNanos);
> -
> - String year = format((getYear() + 1900), 4);
> - String month = format((getMonth() + 1), 2);
> - String date = format(getDate(), 2);
> - String hours = format(getHours(), 2);
> - String minutes = format(getMinutes(), 2);
> - String seconds = format(getSeconds(), 2);
> -
> - return year + '-' + month + '-' + date + ' ' + hours + ':' + minutes
> - + ':' + seconds + '.' + theNanos;
> - }
> + StringBuilder sb = new StringBuilder(29);
>
> - /*
> - * Private method to format the time
> - */
> - private String format(int date, int digits) {
> - StringBuilder dateStringBuffer = new StringBuilder(String.valueOf(date));
> - while (dateStringBuffer.length() < digits) {
> - dateStringBuffer = dateStringBuffer.insert(0, '0');
> + format((getYear() + 1900), 4, sb);
> + sb.append('-');
> + format((getMonth() + 1), 2, sb);
> + sb.append('-');
> + format(getDate(), 2, sb);
> + sb.append(' ');
> + format(getHours(), 2, sb);
> + sb.append(':');
> + format(getMinutes(), 2, sb);
> + sb.append(':');
> + format(getSeconds(), 2, sb);
> + sb.append('.');
> + if (nanos == 0) {
> + sb.append('0');
> + } else {
> + format(nanos, 9, sb);
> + while (sb.charAt(sb.length() - 1) == '0') {
> + sb.setLength(sb.length() - 1);
> + }
> }
> - return dateStringBuffer.toString();
> +
> + return sb.toString();
> }
>
> - /*
> - * Private method to strip trailing '0' characters from a string. @param
> - * inputString the starting string @return a string with the trailing zeros
> - * stripped - will leave a single 0 at the beginning of the string
> - */
> - private String stripTrailingZeros(String inputString) {
> - String finalString;
> + private static final String PADDING = "000000000"; //$NON-NLS-1$
>
> - int i;
> - for (i = inputString.length(); i > 0; i--) {
> - if (inputString.charAt(i - 1) != '0') {
> - break;
> - }
> - /*
> - * If the string has a 0 as its first character, return a string
> - * with a single '0'
> - */
> - if (i == 1) {
> - return "0"; //$NON-NLS-1$
> - }
> + /*
> + * Private method to format the time
> + */
> + private void format(int date, int digits, StringBuilder sb) {
> + String str = String.valueOf(date);
> + if (digits - str.length() > 0) {
> + sb.append(PADDING.substring(0, digits - str.length()));
> }
> -
> - finalString = inputString.substring(0, i);
> - return finalString;
> + sb.append(str);
> }
>
> /**
>
>
>