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 Mon, 01 Sep 2008 16:26:33 GMT
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
>

Mime
View raw message