harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nathan Beyer" <ndbe...@apache.org>
Subject Re: svn commit: r689001 - in /harmony/enhanced/classlib/trunk/modules/sql/src/main/java/java/sql: Date.java Time.java Timestamp.java
Date Tue, 02 Sep 2008 02:52:02 GMT
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 <wuyuehao@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 <ndbeyer@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 <wuyuehao@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 <ndbeyer@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 <wuyuehao@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 <ndbeyer@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 <sean.xx.qiu@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 <nbeyer@gmail.com>:
>> >>>>>> I was referring to a functional test, not a performance
test.
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> On 8/27/08, Sean Qiu <sean.xx.qiu@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 <ndbeyer@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,  <qiuxx@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
>

Mime
View raw message